Skip to content
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

feat: Better selections and moving multiple blocks #1276

Merged
merged 13 commits into from
Dec 3, 2024

Conversation

matthewlipski
Copy link
Collaborator

@matthewlipski matthewlipski commented Nov 25, 2024

💖 This feature is sponsored by DINUM 🇫🇷 and ZenDiS 🇩🇪

moveBlocks

moveBlockUp and moveBlockDown have been renamed to moveBlocksUp and moveBlocksDown as they now work when multiple blocks are selected. As well as this, they now work with columns and the behaviour follows Notion aside from a few cases. Namely, column and columnList blocks in the selection are replaced with just the blocks inside them when moving up/down.

ezgif-5-0c6b9e1637

getSelection

This PR changes the logic for retrieving getSelection().blocks.

Generally, we now return the blocks spanned by the selection at the depth of the shallowest block spanned, aka the shared depth. However, when the block in which the selection starts is at a higher depth than the shared depth, we omit the first block at the shared depth. Instead, we include descendant blocks of the first block at the shared depth, starting at the start block at its original depth, adding the siblings after it, and reducing the depth until we reach the shared depth. This sounds a bit confusing, but basically it's to mimic Notion's behaviour (which you can see when moving multiple selected blocks up/down using Cmd+Shift+ArrowUp/ArrowDown).

Previously, getSelection().blocks would return all blocks spanned by the selection regardless of nesting level.

The code should also be much more performant as it no longer calls doc.descendants.

setSelection

A setSelection method has also been added to the editor. This takes 2 block identifiers for the start and end blocks of the selection and makes the selection span them, as well as any blocks between. The blocks passed must have content of some kind (either "inline" or "table"), as the selection created is always a TextSelection in ProseMirror terms. The block identifiers must also point to 2 different blocks.

Other changes

getPrevBlock, getNextBlock and getParentBlock methods have been added to the editor. These are pretty self-explanatory and share a lot of logic with the existing getBlock. All of these also now work for getting column & columnList blocks, whereas getBlock could previously only find regular blocks.

Fixed an error sometimes being thrown by the table handles when removing blocks. This was specifically happening with moveBlocksUp and moveBlocksDown.

Closes #1239

Copy link

vercel bot commented Nov 25, 2024

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

Name Status Preview Updated (UTC)
blocknote ✅ Ready (Inspect) Visit Preview Dec 2, 2024 11:09am
blocknote-website ✅ Ready (Inspect) Visit Preview Dec 2, 2024 11:09am

- Moved `prevBlock`, `nextBlock`, and `parentBlock` from `Selection` fields to getters on the editor which take a `BlockIdentifier`
- Fixed table handles error being thrown sometimes when removing blocks (was triggering on `moveBlocks`)
@matthewlipski matthewlipski changed the title fix: getSelection returning all descendants fix: Selections and moving multiple blocks Nov 28, 2024
Copy link
Collaborator

@YousefED YousefED left a comment

Choose a reason for hiding this comment

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

Such a great improvement both in UX and affected code!! Really happy with the parts we decided to improve. Definitely a bit more work than expected, but worth it for sure.

Couple of things:

  • Should we close toolbars when moving blocks?
    • Notion doesn't do this so probably not necessary
  • re. the selection created is always a TextSelection in ProseMirror terms -> I guess this is intentional "for now", but let's add a comment that this should be improved? Or in general add a comment (or create an issue) with your thoughts of what a better selection API would look like?
  • lets add a gif (moving through multi column / etc) it looks pretty cool! And "sponsored by dinum / zendis" (both PR and when doing release, check file exporter PR for exact text)

Breaking changes:

  • Are there any breaking changes in this PR for consumers? If so:
    • release should be major
    • make clear what changed (in PR and release notes)
    • see if any docs or examples need textual updating

@matthewlipski matthewlipski changed the title fix: Selections and moving multiple blocks feat: Better selections and moving multiple blocks Nov 29, 2024
@matthewlipski
Copy link
Collaborator Author

matthewlipski commented Nov 29, 2024

Such a great improvement both in UX and affected code!! Really happy with the parts we decided to improve. Definitely a bit more work than expected, but worth it for sure.

Couple of things:

  • Should we close toolbars when moving blocks?

    • Notion doesn't do this so probably not necessary
  • re. the selection created is always a TextSelection in ProseMirror terms -> I guess this is intentional "for now", but let's add a comment that this should be improved? Or in general add a comment (or create an issue) with your thoughts of what a better selection API would look like?

  • lets add a gif (moving through multi column / etc) it looks pretty cool! And "sponsored by dinum / zendis" (both PR and when doing release, check file exporter PR for exact text)

Breaking changes:

  • Are there any breaking changes in this PR for consumers? If so:

    • release should be major
    • make clear what changed (in PR and release notes)
    • see if any docs or examples need textual updating

Re breaking changes - I think it's only that moveBlockUp/moveBlockDown have been renamed to moveBlocksUp/moveBlocksDown.

Re docs - we previously didn't add them for moveBlockUp/moveBlockDown, should we add them now? As for selections, getSelection doesn't need updating but maybe we want to add docs for setSelection (not sure as it's only kind of "good enough" atm).

@matthewlipski matthewlipski merged commit 6f1387e into main Dec 3, 2024
4 checks passed
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.

Can't use ctrl + ⬆️ / ⬇️ if I select multiple blocks
2 participants