Add header to the quick edit when bulk editing #67390
Open
+131
−27
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
|
Size Change: +371 B (+0.02%) Total Size: 1.83 MB
ℹ️ View Unchanged
|
|
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. |
louwie17
requested review from
youknowriad,
oandregal,
jameskoster and
gigitux
|
@jameskoster here is the initial header for bulk editing without the ellipsis action menu. Let me know if you have any styling suggestions. |
jameskoster
reviewed
Nov 28, 2024
| className="edit-site-post-edit-header__title" | ||
| weight={ 500 } | ||
| as="h2" | ||
| lineHeight="20px" |
There was a problem hiding this comment.
Sounds good, I updated that as part of: f35707b
I did have to update the mixins though, as they were relying on a line height variable that didn't exist, let me know if I updated it correctly: https://github.com/WordPress/gutenberg/pull/67390/files#diff-96351000444fb28f5c80a41909d185066c8202d23cdb19c69fc21f79f76960e8L18-R64
Also I did mimic the above from the PostCardPanel component
heading-medium() as well?
| } | ||
|
|
||
| .edit-site-post-edit-header__title { | ||
| padding: 2px 0; |
There was a problem hiding this comment.
Is this to align the icon and title? I think we can do that by adjusting the align property on the HStack, then remove this style.
There was a problem hiding this comment.
Yeah, and thanks for calling out the use of align. Align center allows me to remove the above :)
|
This is working well :) I left a couple of small comments. We might consider using |
|
Random observation, on this branch I'm not seeing the 'Homepage' and 'Posts page' badges next to the relevant page titles. Unsure if it's related to the changes here or not? Seems to be fine on trunk: 
|
youknowriad
reviewed
Nov 29, 2024
| ); | ||
|
|
||
| return { | ||
| icon: unlock( select( editorStore ) ).getPostIcon( postType, { |
There was a problem hiding this comment.
Weird place for such a selector (nothing related to this PR just an observation :P)
youknowriad
reviewed
Nov 29, 2024
| const _record = getEditedEntityRecord( | ||
| 'postType', | ||
| postType, | ||
| ids[ 0 ] |
There was a problem hiding this comment.
Looks like this only depends on the first id, so we might not need the useMemo for ids at all
There was a problem hiding this comment.
Hmm we do still make use of the ids further down, both to decide if to show the single header or to show the count of records being edited.
Or do you mean not passing the entire ids list into the useSelect dependency array?
Btw I am happy to remove useMemo use, as split is a low effort task. I followed what was done in the PostEditForm component ( line ).
I could also pass in the list of ids into the Header component.
youknowriad
reviewed
Nov 29, 2024
youknowriad
reviewed
Nov 29, 2024
| @@ -0,0 +1,84 @@ | |||
| /** | |||
There was a problem hiding this comment.
Nit: I wonder if this deserves its own folder or if it could be just a header.js file within the post-edit.
Personally and given the CSS is smallish, I could have kept it in the same folder.
There was a problem hiding this comment.
Yeah I was toying with that in my head as well. I did move it to the post-edit folder: b4297a7
youknowriad
added
[Type] Enhancement
louwie17
force-pushed
the
add/67344_bulk_editing_header
branch
from
cc7ccb7
to
b4297a7
Compare
|
Thanks for the reviews @jameskoster and @youknowriad, I have addressed the suggestions and this should be ready for re-review.
@jameskoster how do I actually get these badges to show? I wasn't able to display them on |
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?
This partially addresses #67344, by adding a header to the quick edit panel when we are quick editing multiple pages.
Follow ups
PostActions( link ) do we change this to add bulk support?DataViewbulk actions ( link )Why?
In line with the header we show for editing a single page, we should also show one for editing multiple pages.
How?
I created a
PostEditHeadercomponent that handles both the header for a single post/page or multiple.Incase of a single it renders the existing
PostCardPanel.Testing Instructions
Changes will be applied to all selected pages.descriptionTesting Instructions for Keyboard
Screenshots or screencast
quick-bulk-editing-header.mp4