-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Update: Add descriptions to all panels in the Site Editor's dark side #48739
Update: Add descriptions to all panels in the Site Editor's dark side #48739
Conversation
Size Change: +375 B (0%) Total Size: 1.34 MB
ℹ️ View Unchanged
|
@@ -31,35 +31,40 @@ export default function SidebarNavigationScreenMain() { | |||
<SidebarNavigationScreen | |||
isRoot | |||
title={ __( 'Design' ) } | |||
description={ __( | |||
'Customise your Navigation menus, Templates, and more.' |
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.
should this be "and Styles" instead of "and more"?
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.
Yes.
packages/edit-site/src/components/sidebar-navigation-screen-main/index.js
Outdated
Show resolved
Hide resolved
content={ | ||
<ItemGroup> | ||
{ !! navigationMenus && navigationMenus.length > 0 && ( | ||
<> |
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.
We can remove the fragment right ?
packages/edit-site/src/components/sidebar-navigation-screen-templates-browse/index.js
Outdated
Show resolved
Hide resolved
@@ -51,6 +51,11 @@ export default function SidebarNavigationScreenNavigationItem() { | |||
icon={ pencil } | |||
/> | |||
} | |||
description={ | |||
postType === 'page' | |||
? __( 'This is a static page.' ) |
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.
Let's do something that helps explain what pages are, if possible. For example, from https://make.wordpress.org/support/user-manual/posts-vs-pages/#about-pages
"Pages are static and are not listed by date. Pages do not use tags or categories."
"Posts are entries listed in reverse chronological order on the site homepage or on the posts page."
postType === 'page' | ||
? __( 'This is a static page.' ) | ||
: __( 'This is your posts page' ) | ||
} | ||
content={ | ||
<> | ||
{ post?.link ? ( |
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.
Not a big deal, but I think it might be better if the ExternalLink
appears above the description, closer to the title.
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 actually prefer it below, in fact I consider it a little bit of an awkward affordance to have just for some of the types and not others. I might even suggest removing this link — though that's not at all a strong opinion. Rather, I think we need a separate "view on frontend" affordance somewhere, that's general. Might even be a external link icon that appears when hovering or focusing the frame, or a permanent button, something like that.
Flaky tests detected in dfd1c93. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4325475901
|
Took this one for a spin, and it's so much better — this is a crucial piece to land, let's get this in. Matías left a few comments, let's address those. Then there's a question of whether we need to fix the margin and spacing issues mentioned above in this PR, or separately. |
29dbfbd
to
3b36d78
Compare
…in/index.js Co-authored-by: Matias Ventura <mv@matiasventura.com>
3b36d78
to
dd46d7e
Compare
There is some small overlap between this and #48732 |
I applied all the feedback to the descriptions. |
I just cherry-picked this PR to the wp/6.2 branch to get it included in the next release: 976b29e |
#48739)Co-authored-by: Matias Ventura <mv@matiasventura.com> * Update: Add descriptions to all panels in the Site Editor's dark sidebar. * Update packages/edit-site/src/components/sidebar-navigation-screen-main/index.js Co-authored-by: Matias Ventura <mv@matiasventura.com> --------- Co-authored-by: Matias Ventura <mv@matiasventura.com>
Fixes: #48718
Adds descriptions to all browse mode panels.
cc: @jasmussen @jameskoster