[Site Editor]: Move featured image at the top of the inspector controls #59783
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
ntsekouras
added
[Type] Enhancement
ntsekouras
requested review from
youknowriad,
oandregal,
jameskoster,
SaxonF,
jasmussen and
jorgefilipecosta
|
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. |
ntsekouras
commented
Mar 12, 2024
| @@ -142,7 +141,6 @@ export default function TemplatePanel() { | |||
| ) } | |||
| <PostLastRevisionPanel /> | |||
| <PostTaxonomiesPanel /> | |||
| <PostFeaturedImagePanel /> | |||
There was a problem hiding this comment.
@youknowriad you have added this here, but I don't think it's needed or used anywhere..
Am I missing something?
|
Size Change: +71 B (0%) Total Size: 1.71 MB
ℹ️ View Unchanged
|
|
I just shared some initial designs for this in the tracking issue (#59689). In terms of this PR here's the most relevant part for reference (Page inspector): 
Tentatively I separated Title and Summary into dedicated panels, but they could easily be combined if y'all feel that's a better approach. For me that decision will probably depend on whether or not we want to make the Summary panel collapsible or not. That's not something we necessarily have to decide here. Either way, I would suggest moving the featured image to the Summary panel for now so that all editable meta is grouped together. Otherwise this looks good 👍 |
@jameskoster That means revert the I think we could keep them though in this PR too.. |
Which changes do you mean? The main difference I see is the width of the description. I think that's fine and aligns with the designs in the tracking issue. |
That's the only difference, yes. You can also check the screenshot in the PR's description, that shows the change with the So, it seems we can keep this. |
youknowriad
reviewed
Mar 12, 2024
packages/editor/src/components/post-featured-image/with-panel-check.js
Outdated
Show resolved
Hide resolved
youknowriad
reviewed
Mar 12, 2024
packages/edit-site/src/components/sidebar-edit-mode/page-panels/page-summary.js
Outdated
Show resolved
Hide resolved
|
It seems the design (proportions) in the details panel is a bit different than the one in the inspector. Maybe we should use the details panel technique for the width/height of the featured image instead? |
Yes I think so. Areas will likely move to the 'Content' panel anyway. I can work on a follow-up to adjust the title card appearance details to match the designs in the tracking issue.
That seems like a good idea. The proportions in the dark panel are ~12/5 which feels a touch small in the narrower Inspector. Something like |
|
Thanks for working on this. I want to also send a nod to notably @javierarce and @shaunandrews as some of their past designs are finally coming to fruition: #39082 |
ntsekouras
force-pushed
the
move/featured-image-panel-top-of-inspector
branch
from
a5c0426
to
13f17ca
Compare
oandregal
reviewed
Mar 13, 2024
oandregal
reviewed
Mar 13, 2024
| <div className="edit-site-sidebar-card__description"> | ||
| { description } | ||
| </div> | ||
| <HStack spacing={ 3 } className="edit-site-sidebar-card__header"> |
There was a problem hiding this comment.
I'll let designers weight on this. My eye-ruler doesn't like that the icons (title/header vs areas) are not aligned:
| Before | After |
|---|---|
There was a problem hiding this comment.
Happy to make changes in this PR to massage the details so this resembles the designs in #59689 (comment), but that might be better handled in a follow-up?
There was a problem hiding this comment.
Follow up can work for me. No strong opinions
oandregal
approved these changes
Mar 13, 2024
ntsekouras
commented
Mar 13, 2024
| @@ -40,10 +38,6 @@ function ResponsiveWrapper( { | |||
| } | |||
|
|
|||
| const TagName = isInline ? 'span' : 'div'; | |||
| let aspectRatio; | |||
| if ( naturalWidth && naturalHeight ) { | |||
There was a problem hiding this comment.
We should definitely not update this component(components package), but made me notice that we use that in PostFeaturedImage component. We might want to update there or remove this wrapper. I thought it was baked in mediaUpload.
|
Pushed a couple of tweaks:
|
jameskoster
approved these changes
Mar 13, 2024
ntsekouras
force-pushed
the
move/featured-image-panel-top-of-inspector
branch
from
c963e8d
to
b546a2f
Compare
|
@jameskoster thanks for making the changes, but had to revert some of them due to: #59783 (comment). I removed the responsive wrapper completely(details panel also don't have a responsive wrapper), but might still need some polishing, when you find some time. What I noticed though is that the border you added is visible while loading the image. Can we add it to a different element or adjust the styles? |
|
Ah, thank you for catching that.
Using |
jameskoster
reviewed
Mar 13, 2024
|
Pushed one other small tweak to make the 'Set featured image' button dimensions match the image when set.
|
|
Thanks you so much @jameskoster for the polishing! I guess this is ready to land. |
|
Yes I think so! |
carstingaxion
pushed a commit
to carstingaxion/gutenberg
that referenced
this pull request
Mar 27, 2024
…ls (WordPress#59783) Co-authored-by: ntsekouras <[email protected]> Co-authored-by: jameskoster <[email protected]> Co-authored-by: youknowriad <[email protected]> Co-authored-by: oandregal <[email protected]> Co-authored-by: jasmussen <[email protected]>
16 tasks
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?
Part of: #59689
This PR adds the Post Featured Image panel on top of the inspector controls, as part of the normalization with the details panel. The panel checks are preserved and the previous
panelhas been removed.This is the first PR of the main issue and will need design feedback here and will indicate how we will move forward. For example if we merge the
summarypanel in the same container.In order to start this work, I have made some updates to the
SidebarCardcomponent which affects though other panels too. For example in templates where we show the template parts. I think that's the direction we want to go.Testing Instructions
Screenshots or screencast