-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
[Site editor]: Lower emphasis on style variations in navigation sidebar #51964
Conversation
4b95719
to
a3ceb0d
Compare
Size Change: +633 B (0%) Total Size: 1.44 MB
ℹ️ View Unchanged
|
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.
IMO this feels like an excellent stop-gap solution to reducing the prominence of variations while we work out remaining sections (fonts, colors, etc). Here a GIF showing the variations in a deeper drilldown:
I'd love a sanity check by @jameskoster and/or @annezazu / @richtabor
I'm not super-confident about this, but it can probably work as a stop-gap like you mentioned. Would "Edit styles" work better than "Open styles"? It kind of feels like you already 'opened' Styles by drilling into this panel. |
Edit styles can work 👍 👍 One of the reasons I think this can work is that a) it solves the immediate prominence issue while keeping the benefit of the deep-link to global styles, and b) it doesn't preclude the larger considerations from happening, as those would still need an "Edit styles" menu item to exist. |
Was coming to mention similar. "Browse" and "Open" styles seem like two ways to say the same thing really. I propose perhaps "Browse variations" or "Browse theme variations" or just "Theme variations" (as the top-level is not action-based anyhow). And for the second control (still unsure if we want to surface "Global", but perhaps "Global styles".
|
packages/edit-site/src/components/sidebar-navigation-screen-global-styles/index.js
Show resolved
Hide resolved
What's the TLDR of the reasoning behind this PR. At first sight personally I largely prefer the behavior in trunk. |
The original reasoning is in the linked issue #50924 but the TLDR is that switching style variations is unlikely to be a common action users will take and, with the current set of bugs when switching, it feels risky to surface so prominently. |
I agree with the "risky" aspect but I think it's more common to switch style variations than tweak a specific thing like a button color or something (aka accessing specific global styles). (unless you're a theme author ofc) |
It's especially risky given the undo action is buried in this context.
That's probably true when you're initially setting up a site, but once the site is established granular edits are probably going to be more likely? Given that changing variation is pretty much the only edit action you can take in this panel, and that there is a desire to de-emphasise that action, the purpose of the panel is brought into question imo. It seems a bit counter-intuitive to dedicate a whole panel to a feature that we do not want to be so prominent. I still think we should consider deep linking to the main Styles panel from the design root as an alternative here. |
I feel the main issue here is that we probably want to bring the "styles" panel out of "edit" mode because it feels higher level and than since it's maybe technically more challenging to do so (inverted colors), we're coming up with a less than ideal fix. I don't like the unnecessary step in this PR. It feels useless to open "styles" then click another "browse styles" or "browse styles variations" to be able to do anything. |
But it wouldn't be unnecessary. This is how I picture the structure in the future:
We have to remember that site view is ideally about surfacing high level changes you do in a receeded view, which could be to pick between several font pairings, or full color palettes, or indeed style variations. But edit view would still be reserved for changing the individual fonts of a pairing, or the individual colors of a palette, and of course all the global styles that still need to be accessible for a theme builder. That's what I mean about us still having several options exist in the Styles drilldown, alongside a link to editing global styles. So whether those global styles then still take you into edit view with a sidebar on the right, or into an abstracted view like this, that is still a possibility while landing this. |
I don't have a strong opinion on this one, but IMO this is not a common user action. I don't think having a dedicated screen inside a nested menu makes it more prominent. When a user wants that, will have a nice screen to do so and if not, just a menu item. We could possibly move the menu item to be below, but in general I think this iteration achieves to lower emphasis. |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
I tend to agree. In time, the Styles panel could have more functionality (panels) that make sense—like the Font library and perhaps color palettes—but as-is this PR adds additional weight to the flow without much of a gain.
Noting that this is what happens when a theme does not have any theme style variations. It's not bad, but a bit jarring. Overall, I lean towards keeping the styles panel as-is, not de-emphasizing style variations. |
I disagree mainly due to these kinds of bugs present (and the poor empty state) but understand we could iterate in the future: #47433 (comment) & #48407 (comment) |
Just to follow up, let's go with what's outlined in this issue: #52158 |
What?
First version of: #50924
This PR implements the first iteration based on designs from here:
The side effect(not a bad one) is that now we also have a dedicated url path for the style variations.
Testing Instructions
Styles
.Screenshots or screencast
Screen.Recording.2023-06-27.at.12.15.31.PM.mov