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(list): enhance rich text list editing experience #298

Merged
merged 6 commits into from
Mar 28, 2024

Conversation

giamir
Copy link
Contributor

@giamir giamir commented Mar 26, 2024

This PR enhance the editor rich text experience when editing lists.
For more context about the issue see also this document.

How to test

Part 1:

  • Write at least 3 items in 3 different rows (e.g. Item 1 then Enter, Item 2 then Enter and so on)
  • Select all the 3 items
  • Click on the bullet list menu button (or shortcut Meta+u)
  • Observe the items becoming a bulleted list of 3
  • Click on the bullet list menu button again (or shortcut Meta+u)
  • Observe the items being removed from the list
  • Repeat the above steps with the ordered list button (shortcut Meta+o)

Part 2:

  • Add in the editor a bullet list with 3 items
  • Select the 2nd item
  • Click on the bullet list menu button
  • Observe the second item being removed from the list
  • Click on the bullet list menu button again
  • Observe the item being rejoined in a single list

All tests and linters should pass.

STACKS-534

Copy link

netlify bot commented Mar 26, 2024

Deploy Preview for stacks-editor ready!

Name Link
🔨 Latest commit 3c3bea3
🔍 Latest deploy log https://app.netlify.com/sites/stacks-editor/deploys/6604077af41e180008f3a9d6
😎 Deploy Preview https://deploy-preview-298--stacks-editor.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@giamir giamir added the mode - rich text Affects the editor's rich text (wysiwyg) mode label Mar 27, 2024
@giamir giamir marked this pull request as ready for review March 27, 2024 11:46
@giamir giamir requested a review from dancormier March 27, 2024 11:48
Copy link
Contributor

@dancormier dancormier left a comment

Choose a reason for hiding this comment

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

This works as expected (with one very minor quirk noted below) and the code seems reasonable. Thanks for taking care of this @giamir.


Noting one minor quirk

Note

For the record, I don't think this behavior is significant or worth handling, so I don't suggest any changes to this PR.

I took the testing instructions in the PR description very literally and ran into an unexpected behavior.

I added three items (and nothing else), I then selected them all with either ⌘ + a or by clicking and dragging across all of them, and then entered either ⌘ + u or clicked the bullet list button. I then entered either ⌘ + u or clicked the bullet list button again and saw the bullet list get nested inside a new bullet list.

20240327-115042

This only seems to happen when all of the contents of the editor are selected. I imagine this circumstance is probably rare and is not an edge case worth handling, but felt like it was worth noting.

@giamir
Copy link
Contributor Author

giamir commented Mar 28, 2024

This only seems to happen when all of the contents of the editor are selected. I imagine this circumstance is probably rare and is not an edge case worth handling, but felt like it was worth noting.

I noticed this as well. This happen only if you select all the content. I agree with you we can consider this a rare edge case for now. Also I noticed that similar odd issues also happen with other commands when selecting all the content. Probably something that should be tackled in a separate PR anyway (not related to these changes).

Thanks for the review. 🎉

@giamir giamir merged commit 998c820 into main Mar 28, 2024
5 checks passed
@giamir giamir deleted the STACKS-534/enhance-rich-text-list-editing branch March 28, 2024 08:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mode - rich text Affects the editor's rich text (wysiwyg) mode
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants