Editor: Avoid remounts of DocumentBar
#62214
Merged
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
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
stokesman
added
[Type] Bug
|
Size Change: -27 B (0%) Total Size: 1.74 MB
ℹ️ View Unchanged
|
|
This probably changes the header for posts without "title" support, I wonder how it looks now |
|
It looks like that. We should probably hide the icon as well no? 
|
Maybe. Yet perhaps a question of greater importance is whether omitting the I noticed the document sidebar in posts without "title" support looks like this: Simply removing the Not that I think it’s ideal since there’s no distinguishing a post that can’t have title from one that lacks it. I’d like to try improving it but I think we’d need to modify |
stokesman
changed the title
PostTypeSupportCheck in DocumentBarDocumentBar
|
I don't mind that and you make a good point about the lack of H1 but would love a design check at least. @jasmussen @jameskoster |
I guess this is the main question, correct me if I'm wrong. Pages need an H1 so good find, important to fix. Jay is out on vacation for a few weeks, so I don't have the full context he might have, but it seems like an unintentional omission, and including the title, especially because it's in the inspector, seems fine. Though we need to decide what it says. Currently when I add a new post, with title support but no title, it says No Title:
But in these contexts, it says (no title):
I've no strong preference here other than unifying this, which can happen separately from this, but is worth doing. Easiest is to just change it to "(no title)" everywhere. The parenthesis subtly emphasizes that it isn't the title, that it's just placeholder text, but maybe the lack of sentence case is a little weird, perhaps "(No title)" is better? In any case, "No Title" is wrong, we don't use titlecase in these contexts. |
|
@jasmussen The main question for me is whether showing "(no title)" as a title for post types that don't have titles is the way to go. It's a bit different than posts with an empty title. |
Yes, understood. I'm hearing that "(no title)" may still suggest it can hold a title? We could look at other bits of text, though I'd suggest they follow the same format, such "(title disabled)"? I don't love that, to be clear, and I think we should, across all of WordPress, change what these say so it's consistent. But we can flex a bit to get this PR shipping: it's easy, and seems safe, to change this verbiage again on the block editor side of things, so it seems like we should pick something and go with it. |
youknowriad
approved these changes
Jun 4, 2024
youknowriad
added
the
Backport to WP 6.7 Beta/RC
|
Thanks Joen and Riad for weighing in and reviewing ❤️ |
westonruter
added a commit
that referenced
this pull request
Jun 4, 2024
…dd/on-async-directives * 'trunk' of https://github.com/WordPress/gutenberg: (72 commits) Top toolbar: fix half a pixel artifacting of the bottom border. (#62225) Fix UI order for theme.json spacing sizes (#62199) Chore: Simplify a padding style on global styles. (#62291) Editor: Avoid remounts of `DocumentBar` (#62214) Add `default-spacing-sizes` and `default-font-sizes` options for classic themes (#62252) Editor: Cleanup styles and classnames (#62237) Scripts: Pin the @wordpress/scripts version to a version supported by WP 6.5 (#62234) Documentation: Better changelogs for the JSX transform upgrade (#62265) Chore: Simplify a padding style on dataviews. (#62276) MediaUpload: Remove dialog markup on close (#62168) Tabs: Prevent accidental overflow in indicator (#61979) Make edit site pagination buttons accessibly disabled. (#62267) Fix: Remove unused code from dataviews styles. (#62275) Re-enable React StrictMode (#61943) Inserter: Return the same items when the state and parameters don't change (#62263) Update instances of text-wrap: pretty to fall back to balance (#62233) Update: Slotfill documentation samples (links, code, and rephrase). (#62271) Try: Fix mover positioning. (#62226) [Mobile] - Image corrector - Check the path extension is a valid one (#62190) Update: Block styles documentation. ...
ellatrix
pushed a commit
that referenced
this pull request
Jun 11, 2024
* Incorporate PostTypeSupportCheck in DocumentBar * Remove PostTypeSupportCheck from DocumentBar Co-authored-by: youknowriad <youknowriad@git.wordpress.org> Co-authored-by: jasmussen <joen@git.wordpress.org>
ellatrix
pushed a commit
that referenced
this pull request
Jun 11, 2024
* Incorporate PostTypeSupportCheck in DocumentBar * Remove PostTypeSupportCheck from DocumentBar Co-authored-by: youknowriad <youknowriad@git.wordpress.org> Co-authored-by: jasmussen <joen@git.wordpress.org>
patil-vipul
pushed a commit
to patil-vipul/gutenberg
that referenced
this pull request
Jun 17, 2024
* Incorporate PostTypeSupportCheck in DocumentBar * Remove PostTypeSupportCheck from DocumentBar Co-authored-by: youknowriad <youknowriad@git.wordpress.org> Co-authored-by: jasmussen <joen@git.wordpress.org>
ellatrix
removed
the
Backport to WP 6.7 Beta/RC
|
This was cherry-picked to the wp/6.6 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.
What?
Removes the
PostTypeSupportCheckthat is aroundDocumentBar.Why?
Fixes #61875
More
PostTypeSupportCheckcauses its children to remount as it relies ongetPostTypewhich is asynchronous. The change in this PR avoids the remount that interferes with animation.Testing Instructions
Testing Instructions for Keyboard
Same as above, but I can add more detail if needed.
Screenshots or screencast
Currently
document-bar-remount-no-initial-animation.mp4
On this branch
document-bar-initial-animation.mp4