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
Verse: Disable line breaks #52783
Verse: Disable line breaks #52783
Conversation
Size Change: -4 B (0%) Total Size: 1.44 MB
ℹ️ View Unchanged
|
typeInRichText( verseTextInput, 'A great statement.Again', { | ||
finalSelectionStart: 18, | ||
finalSelectionEnd: 18, | ||
} ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've adjusted the tests. Took inspiration for Pullquote tests, where citation also creates a new default block.
gutenberg/packages/block-library/src/pullquote/test/edit.native.js
Lines 48 to 56 in 8aa016e
typeInRichText( citationTextInput, 'A person', { | |
finalSelectionStart: 2, | |
finalSelectionEnd: 2, | |
} ); | |
fireEvent( citationTextInput, 'onKeyDown', { | |
nativeEvent: {}, | |
preventDefault() {}, | |
keyCode: ENTER, | |
} ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The original spirit of this test was to assert the outcome of pressing Return at the end of lines. While this test change results in the snapshot remaining the same and the test passing, the change here communicates that a user might...
- Type
A great statement.Again
, - Move their cursor between
statement.
andAgain.
1, - And press the Return key.
While this is possible, this behavior may be too cumbersome to be the only way to insert line breaks in the Verse block when using the mobile editor. There is no ability to simultaneously press Shift and Return on most mobile OS virtual keyboards.
If needed, we could diverge the mobile editor behavior to avoid this cumbersome outcome, i.e. introduce a packages/block-library/src/verse/edit.native.js
. Hopefully, we could identify common logic to continue sharing between the two files.
However, before doing so, I wanted to pose a question: how should pressing the Return key behave in the Preformatted block? Should Preformatted behave like a "container" block or should it align with Verse, Header, Paragraph? From my testing, Preformatted currently behaves like the former (as Verse did prior to this PR).
Footnotes
-
This cursor movement is communicated in the form of the
finalSelection[Start|End]
options fortypeInRichText
. In light of this this misunderstanding of the test behavior, it may be best to remove/privatize these options and instead rely upon the explicitselectRangeInRichText
. ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given that the Verse block is for content like poetry and lyrics, line breaks should be expected.
There is no ability to simultaneously press Shift and Return on most mobile OS virtual keyboards.
I agree that moving the cursor position is too cumbersome to be the only way to create a line break within a Verse block on mobile. As a possible alternative, we could consider a soft return, where pressing the shift key and then pressing enter on mobile creates a new line within the Verse block. Soft returns are a common pattern in multiline text input on mobile (like messaging apps) that may already be intuitive to users. A hard return (the behavior implemented in this PR) could create a new block, which would minimize divergence between web and mobile behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the great feedback. @stokesman had the same concerns. It's clear to me now that users expect line breaks when pressing Enter or Return in the Verse block.
It would be easier to revert the changes and mark #52773 as wontfix
for now.
Should Preformatted behave like a "container" block, or should it align with Verse, Header, Paragraph? From my testing, Preformatted currently behaves like the former (as Verse did prior to this PR).
The container blocks support this feature differently when two consecutive Enters create a new block. Maybe we can adopt similar logic for some non-container blocks as well.
Cc @annezazu, since you extracted the original issue from the GitHub discussions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If reverting this change on the web is an option, I'd like to add support for that, as I agree with sentiments already shared that users will be expecting/wanting line breaks in the Verse block.
The container blocks support this feature differently when two consecutive Enters create a new block. Maybe we can adopt similar logic for some non-container blocks as well.
I'd also be wary of adding this as a solution as folks may want the freedom of adding blank lines between content in multiline blocks. In the case of the Verse block, for example, a poem may include multiple stanzas that should be separated with a blank line.
I'm afraid I don't have any bright ideas for solving the original problem of more easily exiting the Verse block. Perhaps after hitting return twice, the toolbar could emerge, so that the option of adding a new block is more readily available if users want to take it? Or perhaps there's some other creative approach that could be taken? 🤔 It would be interesting to figure out whether this is a common issue that those using multiline blocks face.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps after hitting return twice
That's exactly what I have would expected. Three times actually.
- Writing: I am writing a verse
Enter
Still in the verse
Thinking: I want a blank paragraph in between.Enter
Still in the verse
Writing: I write the last paragraph in the verse.Enter
Still in the verse
Thinking: Blank paragraph, as aboveEnter
Still in the verse
Thinking: Okay, if I type now it'd be the same as step 3. But I don't type, so let's get out of here.Enter
Out of the verse
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To put it simply: It's not possible to create two blank paragraphs after each other. (At the end of the verse block)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I see that @jameskoster suggested the same below and it can' t be done right now.
However, this should be definitely be done in the future. Is anyone aware of a ticket, or should we utilise this one: #52773 @Mamaduka Could you please reopen it again?
Flaky tests detected in 5716fda. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5608294109
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks George!
I know this is its own thing but should it work more like in Group and Quote blocks? I.e. only exit if the line is empty? That way if someone is actually composing, they don't have to remember to hit shift+enter every line. |
@stokesman, both those blocks are containers, they have separate logic for block split. This one should work like a Paragraph or Heading. |
I get the difference with container blocks. Yet I'm not sure Verse should be treated just like a Heading or Paragraph. Unlike those, Verse is expected to contain line breaks. I don't think this is very high-stakes but feel it'd be better to preserve the insertion of line breaks except for the special case that the it’s the last line and it’s empty. |
Consider also the Code block—should that exit on enter as well or is it different from Verse? |
@stokesman, I see this as a UX improvement. Users expect to exit a block on enter at the end of the text. I will merge this and am happy to revert the changes if we see reports that more folks don't like it.
I'm not sure, to be honest. I usually copy/paste the content into the code block and am okay with the same behavior. This might be the opposite for others. @richtabor, @jameskoster, do you have any thoughts on how we should unify this UX for blocks? |
Could the List pattern work here? IE a single |
That feature only works with container blocks at the moment. We can't enable it here. |
Ah, it wasn't a suggestion for the short-term. Just a potential answer to the question about standardizing the behavior across blocks like Code, Verse, etc., in the future. |
I wasn’t suggesting this shouldn't be done. Just done it in a way that would feel more consistent with how it's been done elsewhere. I'm not left feeling like that was understood. |
Sorry about that, @stokesman. I agree that block-exiting UX should be consistent across the blocks. For now, the behavior of the Verse block will match Heading and Paragraph. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the ping, @Mamaduka. 🙇🏻♂️ I appreciate you raising the mobile editor team's awareness of this change. I left an inline comment regarding the mobile editing experience for us all to consider.
typeInRichText( verseTextInput, 'A great statement.Again', { | ||
finalSelectionStart: 18, | ||
finalSelectionEnd: 18, | ||
} ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The original spirit of this test was to assert the outcome of pressing Return at the end of lines. While this test change results in the snapshot remaining the same and the test passing, the change here communicates that a user might...
- Type
A great statement.Again
, - Move their cursor between
statement.
andAgain.
1, - And press the Return key.
While this is possible, this behavior may be too cumbersome to be the only way to insert line breaks in the Verse block when using the mobile editor. There is no ability to simultaneously press Shift and Return on most mobile OS virtual keyboards.
If needed, we could diverge the mobile editor behavior to avoid this cumbersome outcome, i.e. introduce a packages/block-library/src/verse/edit.native.js
. Hopefully, we could identify common logic to continue sharing between the two files.
However, before doing so, I wanted to pose a question: how should pressing the Return key behave in the Preformatted block? Should Preformatted behave like a "container" block or should it align with Verse, Header, Paragraph? From my testing, Preformatted currently behaves like the former (as Verse did prior to this PR).
Footnotes
-
This cursor movement is communicated in the form of the
finalSelection[Start|End]
options fortypeInRichText
. In light of this this misunderstanding of the test behavior, it may be best to remove/privatize these options and instead rely upon the explicitselectRangeInRichText
. ↩
Thanks, everyone, for the great feedback. I've reverted the changes #52928. It would be great to have a design guide on handling similar behavior based on the block type so that UX is similar across the blocks. |
What?
Fixes #52773.
PR disables line breaks in the Verse block, so when a user presses Enter at the end of the text, it will create a new default block.
Users still be able to create line breaks using Shift + Enter.
How?
Add the
__unstableOnSplitAtEnd
handle to the RichText component.Testing Instructions
Screenshots or screencast
CleanShot.2023-07-20.at.11.40.23.mp4