Feat/toggles

[Liboul]

Intro

This PR adds toggles! (à la Notion)

See Discussion in Top-Requested Features

Disclaimer 1: Although examples in www work, this PR is not production-ready:

1. Some decisions, discussed below, are debatable and require approval by a more knowledgeable maintainer.
2. I had some issues with the typing, nothing critical, but some @ts-ignore need fixing
3. After the architecture is approved, I'll add tests and finalize the documentation

While some of these decisions may be revisited, this PR might be a good starting point for a final implementation.

Disclaimer 2: Tests fail on my computer, but they also fail on main

Disclaimer 3: This is my first PR on this project, so I'll be adding a lot of explanations to smooth the back and forth with the reviewers.

Objective

The objective is to add toggles to Plate, i.e. adding the possibility to expand / collapse a section by clicking on a button located next to a block that is always visible.

Here's a basic demo in Notion:

Screen.Recording.2024-01-28.at.20.32.35.mov

Challenges

Before diving into the proposed implementation, here is a non-exhaustive list of the challenges to keep in mind. Again, the demo is done on Notion:

Challenge 1: Inserting breaks into a toggle

Depending on whether the toggle is currently open, the behaviour is different:

• Open: Should enter into the collapsible part of the toggle
• Closed: Should create a new toggle

Screen.Recording.2024-01-28.at.20.40.41.mov

Challenge 2: Copying a toggle

Copying an open toggle, along with potentially part of the collapsible section, should copy exactly that.
Copying a closed toggle should copy both the visible and the collapsible sections.

Challenge 3: Indenting or moving an element can change whether it is inside the collapsible section of the toggle

Screen.Recording.2024-01-28.at.20.45.42.mov

Challenge 4: When moving the visible part of a toggle, the collapsible section may no longer be inside a toggle, and new elements may now be inside.

Challenge 5: Controlling open state of the toggles

When inserting a toggle, it should ideally be open by default. But when rendering the editable for the first time, it should rather be closed by default (this is debatable).

Challenge 6: When the toggle is closed, the collapsible section should not be visible / selectable

Challenge 7: Toggles can be nested

This means that to determine if an element is visible, ALL enclosing toggles must be open.

Decisions

Decision 0: Element vs attribute

Option 1: Toggles are a new block element, like "Table" or "Heading".
Option 2: isToggle is a new attribute that can be added to any block

Since blocks cannot contain blocks, if toggles are block elements, this implies that an element cannot be both a heading and a toggle. However, allowing all kinds of blocks to be toggles requires a lot of thought on how they interact. Notion chose to make toggles blocks, for instance an element cannot be both a blockquote and a toggle. However, it can be a heading, but that's because in Notion, headings are not blocks, but leafs, which is a sensible decision.

All this considered, I went with option 1. Headings becoming toggles is rather incumbent on the implementation of headings.

Decision 1: Flat vs Nested

Much like lists, toggles could be implemented either as:

• Flat: The toggle component only contains the visible section, and the collapsible section is composed of indented siblings.
• Nested: The toggle component contains both the visible and the collapsible sections.

=> I went with the flat version, mostly because I felt like it would make normalization easier.

In fact, we could very well imagine 2 separate plugins, one for flat, one for nested, much like lists. Therefore, this plugin depends on the plate-indent plugin.

Decision 2: Hiding the collapsible section

I experimented with multiple CSS rules, and found that the following lead to a natural behaviour:

• height: 0; visibility: hidden; on elements enclosed in a closed toggle.
• editor.isSelectable returns false for elements enclosed in a clsoed toggle.

This also plays well with copying (see challenge 2)

Decision 3: The open state

Options wrt. where & how to store that state:

1. In a component state in toggle-element
2. In a React State at the editor level
3. In a pure JS object
4. In a ZustandStore at the editor level

Decision 3bis: Identifying toggles

Identifying which toggles are open requires to identify the toggles by some kind of id. This plugin could implement this idea itself, or it could rely on another plugin, plate-node-id, which is the route I chose. Therefore, this plugin depends on the plate-node-id plugin.

Decision 4: Hiding / Showing elements based on the open state

I had 2 ideas for this:

1. Adding an attribute inToggleIds on elements that are enclosed in one or more toggles, then injecting style attributes.
2. Injecting a wrapper on top of every element.

I started with option 1 but the normalization rules turned out to be too complex for my taste. Even more complex than the normalization rules of the indent-list plugin. Testing edge cases on the indent-list plugin for the purpose of this toggle plugin actually surfaced bugs in the indent-list due to normalization.

So I went with option 2, which might be less performant, but is much easier normalization-wise. For performance, I made sure to compute the enclosing toggles of each element all in one go, in O(nb of nodes), using memoization, so that the cost really isn't too bad.

Decision 5: Re-computing the visibility of elements (see challenge 4)

On every apply operation, if the operation incurs the possibility of elements moving into a toggle or moving out, we trigger a store update so that elements can recompute their enclosing toggles
This may seem like a hack.

Decision 6: Toggles are not indented by default

The rationale is that if we indent a paragraph below a toggle, it should be considered inside the toggle.
This came as a surprise, but indented lists are indented at 1 by default. This means that a list that has not been manually indented would be considered inside a toggle. I added code to handle that specific use case, but other options could be considered. For instance, toggles could also have a default indent of 1, and in order to be considered inside a toggle, an element would need to have an indent of 2. This last option seems less natural.

Hypothesis: Nested blocks?

This I'm not sure of: I made the hypothesis that every block is at the first level of the document. This seems to be true on all the examples of Plate editors I found. This makes it easy to compute the enclosing toggles of every block, as there's a single non-nested list to traverse. The code can be adapted if this hypothesis turns out to be false, by looking at siblings of elements instead of editor.children. (I realize this point might not be crystal clear)

Demo (sound on), commented, in 3 parts

Demo.part.1.mp4

Demo.part.2.mp4

Demo.part.3.mp4

Ideas on how to improve this plugin

1. Adding an option initialValue of open toggleIds and a callback onChange, for instance to store it in local storage to remember the state upon browser refresh
2. Adding an option closedByDefault: false instead of the current default
3. Adding an option to specify how to get the indent value of elements, right now we are relying on this being the default KEY_ELEMENT
4. An option to specify how to get the id of elements, right now we are using the default id attribute.
5. Adding a placeholder, like Notion does, when the toggle has no elements under it
6. Avoid re-rendering the Toggle wrapper on many commands by restricting the commands to only the relevant ones

[changeset-bot]

🦋 Changeset detected

Latest commit: 0d84809

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
@udecode/plate-toggle	Minor
@udecode/plate	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[vercel]

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

Name	Status	Preview	Comments	Updated (UTC)
plate	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jan 31, 2024 5:15pm

[zbeyens]

Suggested change

	- Turn any bock into a toggle:
	- Turn any block into a toggle:

[zbeyens]

Suggested change

	npm install @udecode/plate-indent @udecode/plate-indent-list @udecode/plate-node-id @udecode/toggle
	npm install @udecode/plate-indent @udecode/plate-indent-list @udecode/plate-node-id @udecode/plate-toggle

[zbeyens]

Let's use jotai so we don't need to pass the store here.

[zbeyens]

Suggested change

	export const injectToggleWrapper = (): InjectComponentReturnType => WithToggle;
	export const injectToggle = (): InjectComponentReturnType => WithToggle;

Let's just use the plugin name

[zbeyens]

With jotai, you'll need to add ToggleControllerProvider in renderAboveEditable

[zbeyens]

Somehow, our build does not tree-shake well lodash. You'll need to import fromlodash/last.js

[zbeyens]

Let's use jotai (see my comment)

[zbeyens]

Suggested change

	export const openFutureToggles = (editor: PlateEditor & ToggleEditor) => {
	export const openNextToggles = (editor: PlateEditor & ToggleEditor) => {

[zbeyens]

With jotai, we only have hooks. Using useEffect in the plugin useHooks, you could do:

const [openIds, setOpenIds] = useToggleControllerStore().use.openIds()

useEffect(() => {
  const options = getPluginOptions(editor, ELEMENT_TOGGLE)
  options.openIds = openIds
  options.setOpenIds = setOpenIds
}, [openIds, setOpenIds])

This is a bit redundant but we plan to build a jotai layer in plate-core so we have an integrated store.

[zbeyens]

Could you remove all the templates changes? We need to wait for the release first (CI condition)

[zbeyens]

Wow, thank you for this huge effort! A few comments:

List vs Indent List

The List plugin was the first one to come out. The nested block architecture was reflecting the HTML one. Normalizing was easy but it came with a lot of complexity and bugs about copy/pasting and moving nodes.

The Indent List has less constraints being flat (like in Word). Complexity is much lower but we needed to mimic the HTML list behavior (list-start, style) and handle normalization of siblings, which is not yet 100% covered as you could see it.

Toggle List

This is not part of Notion, but apps like RoamResearch and Obsidian:

2024-01-29.at.12.36.50.mp4

It would be nice we could reuse (part of) the plugin to support that use-case. Since the current plugin is not completely dependent on indent-list, we can create another plugin for the toggle (indent) list when needed.

Decisions

I'll tag each point with ✅ (compatible) or ❌ (not compatible) with toggle list. These are just notes for later. I'm fine with separate plugin design to ease the development

Element vs attribute

Sounds good. We could extend the plugin to support heading as a prop later. isToggle could be used for the toggle list plugin.

❌

Flat vs Nested

Sounds good, see "List vs Indent List"

✅

Hiding the collapsible section

Nice trick, this will enable a lot of other cases like Tabs.

✅

The open state

Notion persists in the local storage (opening the same page in incognito will close all the toggles). You could use jotai-x for that (we try to get away from zustand):

const { useToggleControllerStore, ToggleControllerProvider } = createAtomStore({
  open: atomWithStorage('plate-toggle-open', {})
}, { name: 'toggleController' });

We generally add Controller suffix when it's handling multiple instances, like PlateController.

✅

Hiding / Showing elements based on the open state

✅ Wrapping sounds good.

Re-computing the visibility of elements

✅ Sounds good.

Toggles are not indented by default

See "Toggle List". We should consider using indent 2 only if the plugin is reused for that.

❌ (if toggle has undefined indent)

Nested blocks

Sounds good for now. There can be "structural blocks" at the root, like pages or columns. I'll think to implement editor.isStructural in plate core so we can skip these nodes when true.

✅

Ideas on how to improve this plugin

1. Later, see above point 3
2. Sounds good for demo purpose. Would name it defaultOpen (similar to Radix, default is false).
3. Later is fine
4. Same
5. Same
6. Same, quite important one though.

[zbeyens]

Here is the icon we could use for the toolbar: https://lucide.dev/icons/list-collapse

[Liboul]

Thanks for the review @zbeyens!

The plugin is now using a JOTAI store as you suggested.

I also revised the way the elements subscribe to the toggle store: We no longer customize editor.apply, instead we're just relying on a single style tag, that is subscribed to the store and hides the hidden elements. That's one less level of nesting in the DOM, and this fixes a regression with tables and draggable elements.

IMO only one suboptimal thing remains: The fact that the toggle index (the index that maps an element id to its enclosing toggle ids) is memoized instead of being a state derived from editor.children, which from I understand is also a JOTAI store? Is there a way to do this? Memoization works, but memory could be freed up using a derived state instead.

Finally:

I can't find the ListCollapse icon, I'm guessing this requires an upgrade of lucide.

I reverted the templates changes as you requested, the revert commit is available to be re-reverted later. Documentation still needs some work.

[12joan]

Nice idea, but there are a couple of problems with this approach:

1. This won't work in apps that use a restrictive content security policy that blocks inline style elements (and for good reason—see point 2).
2. The code as it's currently written is vulnerable to CSS injection if a user opens a maliciously crafted document, which can be used to install CSS key loggers on the page.

[12joan]

A better solution might be to rely on derived Jotai atoms to ensure that components only re-render when a specific ID is modified.

[Liboul]

I see 👍 Any idea on how to derive a state from editor.children? It's the missing piece

[zbeyens]

useEditorSelector is the most optimal one

[Liboul]

Alright guys I reverted back to injecting a component in lieu of adding a style tag.
The behaviour hasn't changed.

Now I just want to make sure the injected component only renders when necessary, and for that I want to define a derived state based on 3 things:

1. The element id
2. The openIds set
3. The toggleIndex that only depends on editor.children

And also discard getEnclosingToggleIds to always rely on the index atom

Sounds straightforward, but I fear that the learning curve of jotai and especially jotai-x is a bit too steep.

Help on this would be much appreciated 🙏 🆘

[zbeyens]

Something like?

// (id: string) is wrong, looking into an alternative...
export const isToggleOpenAtom = atom((get) => (id: string) => {
  const openIds = get(toggleControllerStore.atom.openIds)
  const toggleIndex = get(toggleIndexAtom)
  const enclosedInToggleIds = toggleIndex.get(id) || [];

  return enclosedInToggleIds.every((enclosedId) => openIds.has(enclosedId))
})

const isToggleOpen = useToggleControllerStore().get.atom(isOpenAtom)

@12joan

[Liboul]

element would be undefined?

[12joan]

I would suggest something similar to @zbeyens' suggestion, except define the atom inside the component inside a React.useMemo, and put element.id in the dependency array.

[zbeyens]

From this example:

plate/packages/core/src/stores/plate-controller/plateControllerStore.ts

Line 34 in 6d02746

export const usePlateControllerEditorStore = (

export const useIsToggleOpen = (id: string) => {
  const isOpenAtom = useMemo(
    () => atom((get) => {
      const openIds = get(toggleControllerStore.atom.openIds)
      const toggleIndex = get(toggleIndexAtom)
      const enclosedInToggleIds = toggleIndex.get(id) || [];
    
      return enclosedInToggleIds.every((enclosedId) => openIds.has(enclosedId))
    }),
    [id],
  )

  return useToggleControllerStore().get.atom(isOpenAtom)
}

[Liboul]

No luck. This part of jotai-x's doc states that you can derive state from a store created with createAtomStore. In our case the state derives from both toggleControllerStore and plateEditorStore. So I guess that at least the useIsToggleOpen hook should be symmetric in its use of toggleControllerStore and plateEditorStore.

Using your sample @zbeyens, the toggleIndexAtom stays blocked at an empty map, but the openIds atom is updated correctly.

Using return usePlateStore().get.atom(isOpenAtom) instead of return useToggleControllerStore().get.atom(isOpenAtom), the openIds atom stays blocked at an empty array, but the index atom is updated correctly.

I'm so confused 😆

[12joan]

I think this is a limitation of Jotai; you can't create a derived atom that depends on multiple stores. I've definitely encountered this problem myself. My advice would be to use a different hook for each store, and put the one that's least likely to change first to reduce unnecessary re-renders.

[Liboul]

That's a good idea, opening and closing toggles will surely be much less frequent than updating children 👌

I guess that's what you meant but jotai can definitely derive from multiple atoms, it must be jotai-x that stands in the way.

Just tested, it works without and without constant re-renders.

[12joan]

Deriving from multiple atoms works fine, but not when those atoms come from different JotaiStores, which is where the limitation comes from.

[Liboul]

gotcha, I misunderstood 👌

[Liboul]

I'm done with all the review fixes / optimizations. @zbeyens

Next step: Validation by you, and then documentation and tests by me?

[zbeyens]

Awesome, you can go on for the docs/tests and we'll land it asap.

[Liboul]

I'm done with documentation! @zbeyens

Testing the insert of a break deemed too difficult given the current useEffect hack to set properties on the plugin, since this effect doesn't run in tests.

Linting, typechecking, and testing passes in packages/toggle, but fails at the root, I'm guessing because the package needs releasing?

Not sure how you handle yarn.lock changes, I committed my local changes, I can revert them if necessary.

[zbeyens]

@12joan @Liboul What do you think of -> instead of |>? | symbol is a more technical symbol, hard to find for some keyboard layouts. - is also common to list, in case we support toggle list in the future.

If so, we'd need to comment this autoformat rule for the playground:

plate/apps/www/src/lib/plate/demo/plugins/autoformatRules.ts

Line 21 in 0bf36bc

...autoformatArrow,

[Liboul]

@12joan @Liboul What do you think of -> instead of |>? | symbol is a more technical symbol, hard to find for some keyboard layouts. - is also common to list, in case we support toggle list in the future.

-> conflicts with autoformatArrow. Same with => and >>.
But I agree, in my own project it's just > for toggle and | for blockquote. If it were up to me this would be the default in Plate. As you wish really!

[12joan]

@zbeyens We could use +? I don't think that's reserved in markdown. >> would have been my first suggestion if we don't mind disabling autoformatArrow.

[Liboul]

@zbeyens We could use +? I don't think that's reserved in markdown. >> would have been my first suggestion if we don't mind disabling autoformatArrow.

+ makes a lot of sense to me

[zbeyens]

@Liboul I'm also tempted to choose Notion's defaults, but the conflict with markdown (> for quote) bothers me a bit.

@12joan good idea, single character seems better, and it's analog to -. Let's choose +

[Liboul]

+ it is, it's pushed

[Liboul]

For the record, in the markdown spec, + creates an unordered list, though I have never seen it used.

[zbeyens]

@Liboul Opened a PR to fix the CI

[Liboul]

🟢

[Liboul]

Incredible work you're doing, guys @zbeyens @12joan

The dev experience was awesome, even to a newcomer. Thanks!

[zbeyens]

A pleasure to see such quality in this PR, I still think our DX could be improved with better docs and CLI tooling. Wish you the best for your app!