Add stylebook screen for classic themes #66851
Conversation
|
Size Change: +672 B (+0.04%) Total Size: 1.83 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 859de22. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/12248838879
|
tellthemachines
added
[Type] Feature
|
Thanks for the quick turnaround. I like the implementation, especially the reuse of the stylebook component, great stuff. I know it's early days, so I only have high level thoughts that popped up while testing:
|
I just added a commit that makes sure any additional CSS from the Customizer gets loaded in both the stylebook and the editor (it probs should have been added for the editor before now) Background images are doable if we copy this logic (ideally we should extract what is reusable from there into a core function similar to What do you mean by "links" in this context? And is there a list somewhere of what else might be included in "etc."? I should add that whatever themes have provided editor stylesheets for is already included for free, e.g. TT1 with its dark mode and custom background color.
These are good ideas that would be useful for both classic and block theme users! We should explore them as next steps, given we're already having the "long list of blocks" problem with the newly moved-to-the-left stylebook view on block themes. I think eventually the black sidebar could be used as navigation not only for global styles/stylebook sections in block themes, but for stylebook navigation in classic themes too. Perhaps @WordPress/gutenberg-design folks have ideas around that? |
Some themes, e.g., Twenty Eleven allow users to edit site link colors in the Customizer.
I like the sounds of that! |
|
Nice, this is potent. The benefit of having the style book available for classic themes, is that it can become a gateway to adding theme.json properties, which on their own have the benefit of being editor-stylable. Plus, blocks also have styles in classic themes, so seeing them here, perhaps styling them here, has value.
I think there's something to think about here, as far as classic or hybrid themes often offering a lot of theme CSS, and it being confusing to see them missing here. Can we just load the theme CSS file in here, in these cases? If we did, we should probably also add a notice message somewhere that explains this. This is also important as far as setting expectations of what you can edit, thinking here of what additional site editor menu items become available. There should probably be a very strict limitation that only items defined in theme.json have counterpart menu items. I.e. if you define colors in theme.json, you get the color menu, if you define typography, you get typography (and the font library!), and so on. The interesting question becomes the "Blocks" section, which is perhaps a place to start: being able to change the default appearance of your block editor blocks would have value for any theme. But if color, typography, and spacing properties are not defined in a theme.json file, would that mean color, typography, and dimensions panels were not available for the individual blocks to style? Probably, right? Curious about your thoughts. |
tellthemachines
force-pushed
the
try/classic-stylebook
branch
from
431d0d0 to
a62465c
Compare
I'm afraid it's down to the themes to provide those styles in a way that is available to the editor. For instance, Twenty Eleven prints its custom CSS in a Basically anything that the theme defines as editor styles will also appear in the stylebook. |
I think this makes sense. Additionally, we should only show any global styles controls at all if the theme has a theme.json file. Otherwise the stylebook should remain static. This PR doesn't handle adding the global styles interface; the intention here is only to show a static stylebook with the theme styles for now. So, considering that our MVP, what needs doing to get this in shippable form?
On the last point, customizer background images aren't currently loaded in the editor, so this may be a lower priority item that could be worked on as a follow up. Is there anything else I'm missing? |
|
Ok I've updated this to remove the useless buttons and add color palette to the overview: 
If this approach looks reasonable I'll start cleaning up the code! |
|
Just took this for a quick test against the Chaplin theme (a classic theme). This theme comes with "color schemes" you can pick from in the Customizer. As I switched between different color schemes, it properly updated in the Styles section. It makes me wonder how or whether we should represent these different color schemes in the Styles interface or if it should just reflect the current settings (I'm guessing the latter for simplicity :) ): chaplin.theme.test.movVery cool to see either way! |
|
A thought: instead of creating individual menu items, such as Styles and Patterns, that all take you into the immersive view with a mostly empty sidebar, could we simply call the menu item "Design", and include both Styles and Patterns there? 
Mainly on my mind here is #64409 as a followup, which is essentially just one aspect of the Styles section, i.e. "Styles > Typography". CC: @WordPress/gutenberg-design |
tellthemachines
force-pushed
the
try/classic-stylebook
branch
2 times, most recently
from
78b5ca5 to
0d44880
Compare
|
In the mobile layout, when we access the Styles menu, we can't do anything. Is there a way to address this issue?
|
|
@t-hamano I've updated to display the style book only in mobile. |
@jasmussen do we want to move forward with this in this PR? If so, let's make sure we're on the same page about how it should work:
|
tellthemachines
force-pushed
the
try/classic-stylebook
branch
from
c7f6f9b to
9f55b77
Compare
Yeah that's my take on it too! |
I've worked it out. In order to decide whether to show the back button, we rely on usePrevious, which stores a ref. Because the We could potentially use |
|
Ok I pushed a fix based on my above comment. Feedback welcome! If everything else is OK the only thing left todo is add some e2e tests 😄 |
packages/edit-site/src/components/sidebar-navigation-screen-main/index.js
Show resolved
Hide resolved
|
This is working great for me in classic and block themes after the latest update. I like the new Design menu: in my opinion it neatly cordons off Site Editor-like features and gives a little elbow room for future additions. One thing I noticed was that hybrid themes (themes with block templates) don't have the menu item. Is that intentional? I was testing the Empty hybrid theme available in the wp-env test env, e.g., http://localhost:8889/wp-admin/themes.php?theme=gutenberg-test-themes/emptyhybrid 
|
Please ignore this. I have been reminded of the following:
Sorry! |
|
Thank you for this work. 🎉 Let's stay close to the metal on next steps, figuring out whether opt-in or opt-out is the right path forward. @bph do you have any instincts on outreach? Thank you all. |
Let's ask: @WordPress/outreach I also added the question to the #outreach channel |
|
I think if a Classic theme has a theme.json file, opt-out is the right path. The theme has already opted into some block-related functionality. How would you opt-out, though? |
Programmatically speaking, the only thing that comes to mind is 🤔 |
My initial instincts for this feature overall were:
This would essentially be super granular opt-in for each, and implicit opt-out by not adding those. I don't know if it's the ideal approach, but I keep wondering the inverse: why would you want to add a theme.json file and_not have the style book_? As a step further: if you do add a theme.json but we disallow editing styles, what's the use case for this?) I don't mean this question to be confrontational, as noted in a few places I don't have the most strong opinion. But instead of adding several layers of confusing opt-outs, can we just use the existing opt-ins/outs that are implicit by being "the presence of a file", or "the presence of an array in that file"? |
|
Expanding on the above, just chatted with some folks too that only underscores that: if people want the style book even if it does nothing, sure, let's do that. The important part is followup PRs where as noted, if you things to your theme.json, those parts become editable. That's the next task. |
I agree with this idea, especially if opens up the possibility of editing global styles. My assumption is that adding a theme.json to a classic theme is a wilful act, and so opt-in by default can be implied. In my view, it's the same vein of logic that determines whether the UI shows template part editing tools for hybrid themes. In the above comment, I bundled in classic themes without theme.json as potentially wanted to opt-out too.
I can however sympathise with the view that some classic themes may want to only expose existing tools, despite having a theme.json. Theme.json in the case of classic themes, has been a tool of convenience for developers/designers. Adding more styles/theme editing options to the Appearance menu, on top of Customize, etc may not be desirable to some folks. |
|
I'm not against that. But I'd still do it one step at a time, and doing the next step only when it becomes requested. Each addition is a new API that lasts forever, so the fewer we have to add, the better. So what we want to avoid is adding one, and no-one using it. |
Unlinked contributors: acketon. Co-authored-by: tellthemachines <[email protected]> Co-authored-by: youknowriad <[email protected]> Co-authored-by: ramonjd <[email protected]> Co-authored-by: t-hamano <[email protected]> Co-authored-by: jasmussen <[email protected]> Co-authored-by: annezazu <[email protected]> Co-authored-by: mtias <[email protected]> Co-authored-by: carlomanf <[email protected]> Co-authored-by: daveloodts <[email protected]> Co-authored-by: cbirdsong <[email protected]> Co-authored-by: fabiankaegy <[email protected]> Co-authored-by: mrwweb <[email protected]> Co-authored-by: ltrihan <[email protected]> Co-authored-by: masteradhoc <[email protected]>
What?
Starts addressing #41119.
Adds a "Styles" item to the Appearance menu when a classic theme is enabled, linking to a site editor page showing the stylebook.
Filters
wp_die()to allow the new site editor URL to be accessed by classic themes (temporary, until we can make the change in core)Adds a
StyleBookPreviewcomponent that renders the stylebook as a preview without being wrapped in an editor.The idea for now is to show a static stylebook for classic themes whether or not they have theme.json support. Building on that later I guess we could show the stylebook + global styles if the theme has theme.json, or perhaps make it an optional theme support.
Testing Instructions