Make default block appender appear more consistently. #28529
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ZebulanStanphill
added
[Type] Enhancement
ZebulanStanphill
requested review from
youknowriad,
ellatrix and
jorgefilipecosta
|
Size Change: +7 B (0%) Total Size: 1.38 MB
ℹ️ View Unchanged
|
ZebulanStanphill
requested review from
ajitbohra,
nerrad and
ntwb
as code owners
|
The
This is what happens instead:
It takes a total of 4 down arrow presses to get out of the Columns block when before it took just 1. Technically, though, this isn't a new problem. It was already happening with every other block... it just wasn't happening with the Paragraph block before this PR. So what should we do? |
|
Thanks for your work. High level, this seems acceptable for most cases. Paragraphs before / after:
Paragraphs in a group, before / after:
Even the consistency added to differing contents in columns seems worth the added consistency, despite the jump:
The jump that happens in the default cover block behavior bugs me quite a bit, though. Before:
After:
I'm aware that it's an unfortunate configuration of the vertical alignment, and the serendipity of the paragraph not being followed by a sibling inserter by default (you have to press Enter like in text editors). But exactly because the Cover is so widely used, while consistent, it feels very jarring. I know I've seen some separate conversations on how to solve this shift, for example by making that appender float, but the heuristic is a little tricky and I don't know if it's solvable easily. So I think we have to review this one on the assumption that this is how it's going to be. And for that reason, I'll need to sleep on it, but would love to hear from others on this! Thanks for your work. |
|
@shaunandrews I am thinking you might want to take a look at this PR. |
ZebulanStanphill
force-pushed
the
update/make-default-block-appender-consistent
branch
from
8a634ff
to
116790d
Compare
ellatrix
reviewed
Mar 9, 2021
| getDefaultBlockName(); | ||
| const isLastBlockValid = isBlockValid( ownProps.lastBlockClientId ); | ||
| getDefaultBlockName() && | ||
| getBlockAttributes( ownProps.lastBlockClientId )?.content === ''; |
There was a problem hiding this comment.
Should we use isUnmodifiedDefaultBlock here?
There was a problem hiding this comment.
(No guarantee that the default block has the content attribute and that it means the blocks is empty.)
There was a problem hiding this comment.
Now that I think about it, perhaps I should only be checking for the Paragraph block specifically? The whole point of this conditional is to handle the case where an empty Paragraph shows an appender inside it. Is that behavior unique to the Paragraph?
|
Closing in favor of #36656. |
ZebulanStanphill
deleted the
update/make-default-block-appender-consistent
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Fixes #5055. Also addresses some points raised in #13571 and #26404.
This PR makes the default block appender more consistent in when it appears.
Current behavior in
masterThe appender appears after every block except a Paragraph block. However, this has often been confusing and annoying in situations such as a Column where inserting a block after the last one in the Column feels weird because you have to do one of the following:
None of these are particularly obvious at first. And the fact that the appender appears for every other block just makes it more confusing. I don't think most users will understand why a Heading or List at the end of a Column or post shows an appender, but a Paragraph doesn't.
Behavior in this PR
The appender appears after every block including the Paragraph, with the exception that empty Paragraph blocks retain the current behavior in order to avoid having multiple appenders shown simultaneously.
I believe this behavior makes a whole lot more sense, and is a lot more predictable for users. I think there's definitely more work that could be done in the future (see #13571), but for now, this PR should greatly improve the situation.
Screenshots
Checklist: