Skip to content

feat: Toggle blocks #1707

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 23 commits into
base: main
Choose a base branch
from
Open

feat: Toggle blocks #1707

wants to merge 23 commits into from

Conversation

matthewlipski
Copy link
Collaborator

@matthewlipski matthewlipski commented May 21, 2025

This PR adds toggle blocks, which have a button in them to show/hide their children.

TODO:

  • Add backspace handling on last empty child block
  • Review & revise styling

Copy link

vercel bot commented May 21, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
blocknote 🔄 Building (Inspect) Visit Preview Jun 12, 2025 2:12pm
blocknote-website ✅ Ready (Inspect) Visit Preview Jun 12, 2025 2:12pm

Copy link

pkg-pr-new bot commented May 21, 2025

Open in StackBlitz

@blocknote/ariakit

npm i https://pkg.pr.new/TypeCellOS/BlockNote/@blocknote/ariakit@1707

@blocknote/code-block

npm i https://pkg.pr.new/TypeCellOS/BlockNote/@blocknote/code-block@1707

@blocknote/core

npm i https://pkg.pr.new/TypeCellOS/BlockNote/@blocknote/core@1707

@blocknote/mantine

npm i https://pkg.pr.new/TypeCellOS/BlockNote/@blocknote/mantine@1707

@blocknote/react

npm i https://pkg.pr.new/TypeCellOS/BlockNote/@blocknote/react@1707

@blocknote/server-util

npm i https://pkg.pr.new/TypeCellOS/BlockNote/@blocknote/server-util@1707

@blocknote/shadcn

npm i https://pkg.pr.new/TypeCellOS/BlockNote/@blocknote/shadcn@1707

@blocknote/xl-ai

npm i https://pkg.pr.new/TypeCellOS/BlockNote/@blocknote/xl-ai@1707

@blocknote/xl-ai-server

npm i https://pkg.pr.new/TypeCellOS/BlockNote/@blocknote/xl-ai-server@1707

@blocknote/xl-docx-exporter

npm i https://pkg.pr.new/TypeCellOS/BlockNote/@blocknote/xl-docx-exporter@1707

@blocknote/xl-multi-column

npm i https://pkg.pr.new/TypeCellOS/BlockNote/@blocknote/xl-multi-column@1707

@blocknote/xl-odt-exporter

npm i https://pkg.pr.new/TypeCellOS/BlockNote/@blocknote/xl-odt-exporter@1707

@blocknote/xl-pdf-exporter

npm i https://pkg.pr.new/TypeCellOS/BlockNote/@blocknote/xl-pdf-exporter@1707

commit: 00fbdae

Copy link
Contributor

@nperez0111 nperez0111 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I'm more interested in seeing here is how we can add this to existing block types (such as headings with a prop) rather than as a separate element like this implements.

Comment on lines 8 to 22
export const ToggleHeading = createBlockSpec(
{
...headingConfig,
type: "toggleHeading",
},
{
...headingImplementation,
render: (block, editor) =>
createToggleWrapper(
block,
editor,
headingImplementation.render(block, editor),
),
},
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fantastic, composability!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now, this is creating a new block type, can we instead just "wrap over" heading, and if it has a prop isToggleable it switches it's render implementation from the default, into using this toggle wrapper?

So that there is only one heading implementation.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I also revert the heading block to using createStronglyTypedTiptapNode? Since the keyboard shortcuts and input rules won't work if we stick with createBlockSpec. Or do you wanna see how it looks using the createBlockSpec first?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now, it is more important to see that the pattern can work before being caught up in those sort of details.

Once we can see that it works for headings, we can see if we can do it for something like list items

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, updated the code👍

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, this looks about right. Ideally it'd be "around" the heading component, but that is minor and fixable.

To explain more, like this you made this:

function Heading(props){
function renderHeading(){}

return props.isToggleable ? <Toggle>{renderHeading()}</Toggle> : renderHeading()
}

blocks: { heading: Heading }

Where I'd like something more like this:

function Heading() {

return renderHeading()
}

const WrapWithToggle=(component) => props => {
return props.isToggleable ? <Toggle>{component}</Toggle> : component
}

blocks: { heading: WrapWithToggle(heading) }

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that we've validated this approach, I do think you have the right idea with making this a strongly typed tiptap node to keep all previous functionality.

Copy link
Collaborator Author

@matthewlipski matthewlipski Jun 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented your suggestions, think it looks pretty decent. There are 2 things that aren't super nice though:

  1. Headings now always need to include a node view, even if they're not togglable.
  2. Because createStronglyTypedTiptapNode works on the TipTap API level while createToggleWrapper works on the BlockNote API level, we have to convert from a node to a block in the node view and do some slightly weird stuff there.

How much of a concern are those 2?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. should be fine, node views are essentially "ejecting" from normal rendering but shouldn't be different (if in pure JS not react)
  2. Yea, so the ideal here is to build everything in terms of the blocknote API instead in the future, just don't wanna invest into that at the moment. May be worth extracting into something common, but not necessary for the moment either.

So, I think this is good from here. 🚦 green light to implement the lists & other ones, it seems like a sound approach for the moment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants