feat: improve collapsible blocks #154

sehyod · 2023-06-13T17:05:00Z

Fixes #139

Typing > now creates a collapsible (or transforms the current block into a collapsible block)
The arrow's colour becomes lighter when the content is empty
Pressing Backspace at the beginning of a collapsible block removes the block
Pressing Enter from collapsed block creates a new collapsible block below
Pressing Enter from uncollapsed block add an empty block in its content
Pressing cmd+Enter from a collapsible block's header toggle between collapsed/uncollapsed states

This is a WIP branch, the behaviours when pressing Enter and when deleting content across nodes containing a collapsible block still need to be defined.

hammer · 2023-06-14T11:16:01Z

Nice! My Notion-like editor experience dreams are becoming a reality.

Could you say more about the situation of deleting content across nodes containing a collapsible block? I'm not sure I understand the situation well enough to comment. When in doubt, try it in Notion!

A few differences I notice from Notion:

In Notion, if my cursor is at the end of the text of a collapsible section and I hit cmd+enter, it opens the collapsible section, and if I hit cmd+enter again, it toggles it back to collapsed. If I hit enter, I get a new, empty collapsible section:

Screen.Recording.2023-06-14.at.1.06.18.PM.mov

In Notion, if I hit tab, the item indents, and if I hit shift+tab, it un-indents.
In Notion, Esc highlights the block and allows the keyboard to be used to move the block up and down: Esc should toggle between block browsing and text editing #151

I also hit what I believe to be a bug: I have an unordered list inside of a collapsible element. When the cursor is at the end of the collapsible element text, I hit enter. Instead of bumping the list down and allowing me to edit directly above it, it just deletes the unordered list.

Screen.Recording.2023-06-14.at.1.13.46.PM.mov

sehyod · 2023-06-14T11:48:55Z

Thank you for the feedback!

What I meant for the situation of deleting across nodes was this situation:

Screen.Recording.2023-06-14.at.12.38.33.mov

What happens in notion is that the content of the last collapsible block is added as the content of the collapsible block where the start of the selection is.

But this actually brings another question: what should happen when the start of the selection is not in collapsible block?

This is the result on notion and it seems a bit weird: as you can see the empty collapsible block is moved above 1.1.3 for some reason:

Screen.Recording.2023-06-14.at.12.40.07.mov

For the cmd+enter, that should be pretty easy to add! Thank you for letting me know about the shortcut :)
We currently don't have any indentation concept: collapsible blocks are blocks with a header and multiple draggable blocks as content. I think we still need to get more comfortable with prosemirror's api to understand what is achievable or not
Thanks for opening the issue about node browsing!
About the bug, it's because the behaviour of pressing Enter has not been implemented in this PR yet, so the results is quite weird. From what I have seen on notion, pressing enter when the node is uncollapsed should add a new empty section, as you mentioned, and pressing Enter on a collapsed node, should add a new collapsible node below. Is that correct?

hammer · 2023-06-14T13:47:39Z

This is the result on notion and it seems a bit weird

That is weird! I don't have a great thought for the right behavior in this situation. The "ghost" collapsible caret with the text dangling below it doesn't seem ideal but maybe there are deeper reasons Notion does that. I think we're fine if we write down the decision we make and file it away to revisit later.

From what I have seen on notion, pressing enter when the node is uncollapsed should add a new empty section, as you mentioned, and pressing Enter on a collapsed node, should add a new collapsible node below. Is that correct?

Sounds correct to me.

…le-blocks

danvk · 2023-06-15T18:42:19Z

Just played around with this a bit. The copy/paste behavior with folds is nice!

A few remaining differences I noticed from Notion, probably not all related to this PR:

Typing should hide the drag handle until you move the mouse again
When you create a fold by typing > at the start of a line, it should be open by default.
When you type > at the start of a bulleted list item, it shifts down to the next line rather than turning the list item into a fold as Notion does:

Screen.Recording.2023-06-15.at.2.25.34.PM.mov

When you hit Enter at the end or the middle of a list item, it should add a new list item, rather than an item outside the list.

Screen.Recording.2023-06-15.at.2.31.24.PM.mov

Backspacing over the "•" at the start of a bulleted list item should remove that item the bulleted list (or move it onto the previous item in the bulleted list if it's not empty):

Screen.Recording.2023-06-15.at.2.27.15.PM.mov

In Notion, the content under a fold isn't indented. Only the fold caret itself is:

vs. in RefStudio the content and the caret are indented:

When you select text, there are broken rectangles for lines that aren't completely selected (the blue under "Ite" on the first line):

Notion just selects the text, rather than the full width of the editor for each line. This looks neater.

Typing ## heading<enter> leaves you in H2 style in RefStudio whereas it drops you back into paragraph style in Notion (which is the better behavior).
Selecting three lines of text and clicking the "Bulleted List" button in the toolbar should make a bulleted list with three items (it currently does nothing).

danvk

This is definitely a strong move in the direction of Notion parity!

The code for manipulating the nodes is quite intricate and my main question is around how we want to set up testing for it. A few ideas:

Have a "script" that a human periodically goes through ("type ABC, put the cursor between B and C and hit enter"). This would be tedious but would at least document our expectations. This is kind of what @hammer and I are doing already in the comments :)
Look into some kind of test runner like Playwright. These sorts of things are always a bit of a PITA to set up but they do create an extremely realistic test environment. I believe this is what VS Code uses. TipTap looks like it uses Cypress, which is similar.
Try to create some sort of unit test by creating the nodes that tip tap expects (maybe by writing out Markdown) and calling the individual functions like setCollapsibleBlock with each situation (no selection, selected text on the fold header line, selection crossing the header line and foldable content). Depending on how TipTap works internally, this may or may not be possible

src/TipTapEditor/CollapsibleBlock/helpers/unsetCollapsibleBlock.ts

src/TipTapEditor/CollapsibleBlock/helpers/unsetPartiallySelectedCollapsibleBlocks.ts

src/TipTapEditor/CollapsibleBlock/nodes/CollapsibleBlockNode.ts

danvk · 2023-06-15T18:59:45Z

src/TipTapEditor/CollapsibleBlock/nodes/CollapsibleBlockNode.ts

+          if (dispatch) {
+            tr.setNodeAttribute(collapsibleBlockStartPos, 'folded', !folded);
+            if (!folded) {
+              // Reset selection if it was inside the content


I hadn't really considered how toggling a fold would interact with selections but… wow!

Yes, I hadn't considered it either, before playing around with the editor. The choice here is to place the caret at the end of the collapsible block's header when the selection overlaps the block content. That's not what notion does, but I think it's a viable solution, that is too hard to implement.

hammer · 2023-06-18T14:18:34Z

I pulled @danvk's comments on Notion parity into a new issue: #154 (comment). Feel free to address the relevant ones here and do the rest in a future PR!

…le-blocks

…sible blocks

cguedes · 2023-06-21T14:55:03Z

You can improve the BACKSPACE behaviour when editing the collapsible block to match Notion.
A BACKSPACE action in the middle collapsible content does what you expect (remove the char), but it you do that in the very end of the content it should create a new block a the parent level. This would allow you to continue writing after the collapsible if is the last one in the document.

Screen.Recording.2023-06-21.at.15.49.42.mov

Additionally you can also implement the SHIFT+TAB action to "remove 1 indentation" when used in a collapsible block.

cguedes · 2023-06-21T15:01:16Z

Collapsible block content is indented in RefStudio. Notion doesn't do that and I agree that we should'n also.

Add'l, I feel that the spacing between blocks (e.g collapsible child) is a bit to big.

cguedes · 2023-06-21T15:03:43Z

Not sure this is the expected behaviour, but the > trigger to create a collapsible don't work for headers

cguedes · 2023-06-21T15:17:47Z

src/editor/nodes/collapsibleBlock/inputRuleHandlers/chevronHandler.test.ts

+    expect($from.sameParent(editor.state.doc.resolve($from.pos - 1))).toBe(false);
+  });
+
+  test('should do nothing when the block cannot be turned into a collapsible block', () => {


why can't we turn headings into collapsible? @sehyod

This is because headers are blocks, whereas collapsible summary must be inline content. I have checked on notion and this is aligned with what they do (you cannot turn a header into a collapsible block)

That is only true for H1 (the page title) in Notion.

hammer · 2023-06-21T15:19:53Z

We have follow up issues in case it’s preferable to get this PR in and address some of the other issues later #168 and #169

cguedes · 2023-06-21T15:32:12Z

You are right @hammer. We will move these comments to the other issue(s) and merge this (it's actually merged already 🎉 ).

sehyod · 2023-06-21T15:38:05Z

Thank you for all the comments! I have addressed the easiest remaining issues and have added tests.

The issue with indentation requires more work and a rework of the schema
Turning a header into a collapsible block is only blocked for h1 on notion, so we should enable it for other heading levels too, but this requires a rework of the schema too, to allow having a block as the collapsible content
Finally, the item lists are broken because this is still the basic TipTap node, which does work properly with our custom nodes (draggable, collapsible, ...) we will need to extend the default list item node to adapt it to our editor

sehyod added 4 commits June 13, 2023 13:47

feat: set collapsible block when typing >

2c3cd1e

feat: remove collapsible content when empty

f311d73

feat: implement unsetCollapsibleBlock

8b4d454

fix: run prettier

150cf0e

cguedes assigned sehyod Jun 14, 2023

fix: unselect content when collapsing a block

e246bbd

sehyod added 11 commits June 14, 2023 15:19

chore: move logic from component to node definition

9a49228

feat: add cmd+enter shortcut

06396ca

chore: use command in collapsible block component

b643260

feat: implement Enter behaviour for collapsible blocks

e0cf099

fix: properly remove last content element of collapsible block

2597284

Merge remote-tracking branch 'origin/main' into 139-improve-collapsib…

6320a2f

…le-blocks

fix: fix behaviour when pressing backspace in collapsible content

c060055

fix: backspace on non empty content blocks

d1dd021

fix: pressing backspace with non empty selection work as expected

7ae59f7

Merge remote-tracking branch 'origin/main' into 139-improve-collapsib…

47cfbb5

…le-blocks

feat: implement Enter handle when text is selected acros nodes

e401906

danvk reviewed Jun 15, 2023

View reviewed changes

hammer mentioned this pull request Jun 18, 2023

Notion parity #168

Closed

This was referenced Jun 18, 2023

Testing node manipulation #169

Closed

Cmd+enter doesn't work as expected in the editor #55

Closed

sehyod added 4 commits June 19, 2023 14:59

feat: make collapsible uncollapsed by default

5f20f7a

feat: enter always creates paragraph

9e3a9f2

fix: address review comments

178c330

Merge remote-tracking branch 'origin/main' into 139-improve-collapsib…

1316a39

…le-blocks

sehyod added 15 commits June 20, 2023 11:57

chore: add tests for collapsible block commands

93ab4ac

Merge remote-tracking branch 'origin/main' into 139-improve-collapsib…

e8c573b

…le-blocks

fix: run prettier

6cda8cd

fix: make collapsible block arrows sharper

79f3cf7

chore: add tests for CollapsibleBlock component

da311ac

chore: clean up tests for collapsible nodes

37e0b64

fix: correctly return result when unsetting partially selected collap…

9682e4f

…sible blocks

fix: incorrect check in test

5ba6c08

chore: add tests for collapsible block shortcuts

2033dda

chore: make directories camel case

daa11de

fix: run prettier

8c37f3f

chore: add tests for backspace command

a9eabf8

feat: overload deleteSelection command

cce1a9e

fix: create command to delete non empty selection

bdcd5ed

fix: do not remove > when block cannot be turned into collapsible

fbf771e

sehyod marked this pull request as ready for review June 21, 2023 09:37

sehyod requested a review from cguedes June 21, 2023 09:38

fix: remove debug shortcut

945f23f

cguedes reviewed Jun 21, 2023

View reviewed changes

cguedes approved these changes Jun 21, 2023

View reviewed changes

sergioramos merged commit 46c2733 into main Jun 21, 2023
11 checks passed

sergioramos deleted the 139-improve-collapsible-blocks branch June 21, 2023 15:30

hammer mentioned this pull request Jun 22, 2023

Examine arrow icon sharpness for collapsible sections #166

Closed

danvk mentioned this pull request Jun 22, 2023

Rework some editor tests to use visual cursor and snapshots #186

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: improve collapsible blocks #154

feat: improve collapsible blocks #154

sehyod commented Jun 13, 2023 •

edited

Loading

hammer commented Jun 14, 2023

sehyod commented Jun 14, 2023 •

edited

Loading

hammer commented Jun 14, 2023

danvk commented Jun 15, 2023

danvk left a comment

danvk Jun 15, 2023

sehyod Jun 19, 2023

hammer commented Jun 18, 2023

cguedes commented Jun 21, 2023

cguedes commented Jun 21, 2023

cguedes commented Jun 21, 2023

cguedes Jun 21, 2023

sehyod Jun 21, 2023

cguedes Jun 21, 2023

hammer commented Jun 21, 2023

cguedes commented Jun 21, 2023

sehyod commented Jun 21, 2023

feat: improve collapsible blocks #154

feat: improve collapsible blocks #154

Conversation

sehyod commented Jun 13, 2023 • edited Loading

hammer commented Jun 14, 2023

sehyod commented Jun 14, 2023 • edited Loading

hammer commented Jun 14, 2023

danvk commented Jun 15, 2023

danvk left a comment

Choose a reason for hiding this comment

danvk Jun 15, 2023

Choose a reason for hiding this comment

sehyod Jun 19, 2023

Choose a reason for hiding this comment

hammer commented Jun 18, 2023

cguedes commented Jun 21, 2023

cguedes commented Jun 21, 2023

cguedes commented Jun 21, 2023

cguedes Jun 21, 2023

Choose a reason for hiding this comment

sehyod Jun 21, 2023

Choose a reason for hiding this comment

cguedes Jun 21, 2023

Choose a reason for hiding this comment

hammer commented Jun 21, 2023

cguedes commented Jun 21, 2023

sehyod commented Jun 21, 2023

sehyod commented Jun 13, 2023 •

edited

Loading

sehyod commented Jun 14, 2023 •

edited

Loading