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

Keeping heading style when prior block is empty on merging #53990

Closed

Conversation

bangank36
Copy link
Contributor

@bangank36 bangank36 commented Aug 28, 2023

What?

Address the issue on #16436 cc: @ellatrix

Why?

  • When Pressing backspace on Heading block, it is merged with the empty prior paragraph where its attribute will be replaced with Paragraph attribute, therefore Heading is lost and replaced with regular Paragraph

How?

  • Check for the content of the blocks and remove the previous block in case it has empty content
  • Assume there are 2 blockA and blockB in the editor, caret is place at start of blockB
    • ➖ is empty text
    • 🆎 is non-empty text
blockA blockB Actions
Merge B to A
🆎 Remove A
🆎 Merge B to A
🆎 🆎 Merge B to A

Testing Instructions

  • Insert an empty paragraph
  • Insert a Heading
  • Place caret at the Heading start and press backspace
  • Expect the paragraph is removed and the heading is kept with its style

E2E testings

  • Introduce new test suite should not merge heading with empty paragraph block on backspace
  • Run against npm run test:e2e:playwright -- test/e2e/specs/editor/various/splitting-merging.spec.js
  • Test failed on should gracefully handle if placing caret in empty container
    • Reason: the backspace removes all empty paragraphs, making the snapshot mismatch

Testing Instructions for Keyboard

Screenshots or screencast

Kapture.2023-09-04.at.22.50.49.mp4

@bangank36 bangank36 force-pushed the fix/empty-p-merges-heading branch from d4f87fb to c1110f2 Compare September 4, 2023 15:23
@bangank36 bangank36 marked this pull request as ready for review September 4, 2023 16:04
@ellatrix ellatrix added [Type] Enhancement A suggestion for improvement. [Feature] Writing Flow Block selection, navigation, splitting, merging, deletion... labels Sep 7, 2023
@ellatrix
Copy link
Member

ellatrix commented Sep 7, 2023

Looking good, just needs some polish :)

@bangank36
Copy link
Contributor Author

Looking good, just needs some polish :)

Thanks for the thorough review, I made some changes and asked for clarification on #53990 (comment)

@bangank36 bangank36 requested a review from ellatrix September 8, 2023 02:15
@bangank36 bangank36 force-pushed the fix/empty-p-merges-heading branch from 39b5a2a to 8406a55 Compare September 14, 2023 09:45
@bangank36
Copy link
Contributor Author

Hi @ellatrix, just a friendly nudge to request your review of the latest changes on my PR when you have a moment. Thanks!

@bangank36
Copy link
Contributor Author

Closed as resolved in #55134

@bangank36 bangank36 closed this Oct 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Writing Flow Block selection, navigation, splitting, merging, deletion... [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants