Edit/create shadows in global styles #60706
Conversation
|
Size Change: +3.3 kB (+0.19%) Total Size: 1.75 MB
ℹ️ View Unchanged
|
madhusudhand
added
[Type] Feature
madhusudhand
force-pushed
the
shadows-editor
branch
from
8d1ae96
to
fd8d6e8
Compare
|
@madhusudhand as promised I've pushed a different approach to parsing shadow strings in 60c40f3. While that commit gets all 2040 unit tests passing, it was only quickly hacked together and could definitely do with some refinement. That said, hopefully, it helps unblock the rest of the UI work here, I'm looking forward to seeing the end result! Some aspects of the updated approach that need looking at are;
There's probably more that I haven't noted but as this PR is still in its early days, I'm sure it will all come out in the wash 😄 |
|
Flaky tests detected in 44a72d0. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/9173073571
|
madhusudhand
force-pushed
the
shadows-editor
branch
from
60c40f3
to
dabd706
Compare
|
@aaronrobertshaw Thanks for the commit. It is working great for all the listed scenarios of valid shadow strings.
I have considered adding few invalid strings such as Our current approach picks all length values and considers first four of them. I am going to refine the regular expression to pick up the units in order. |
Short summary for design review@jameskoster @WordPress/gutenberg-design Changes are now ready for the first design review. While it addresses everything as per the suggested designs, following are done differently. Popover
❓ What do you suggest as the boundaries for the slider. Preview
A white background is used by taking some inspiration from the block preview component. Shadow options
❓Could you suggest designs for the Shadow item labeling
❓When the inset is true, do you think the above labeling makes sense? Shadows panel copy
Please provide copy. |
|
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 Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @jarekmorawski. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. 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. |
madhusudhand
changed the title
Would it make sense to use the AlignmentMatrixControl component before we build a proper one? |
|
Thanks for the ping @madhusudhand, excellent work here, this is coming together nicely :)
Good idea, makes sense.
Also fine, and no rush on this.
0 - 20px seems a reasonable range to begin with – it's easy to tweak this later. I wonder if it's worth capturing negative values? Maybe we try
I think it's worth trying the transparent texture so that white shadows are somewhat visible.
Could we use DropdownMenu V2 for this menu? Rename should open a 
Delete should open a ConfirmDialog, same as deleting a template: 
Important: Only user-generated (custom) shadows should be rename-able or delete-able. Duplicate is a little tricky because we need to think about where to send the user post duplication. I'd be tempted to leave duplication out of this PR, and revisit it separately.
Yup, looks good :)
Maybe something like: "Manage and create shadow styles for use across the site.". Some additional points:
|
Could be worth a try, but probably follow-up territory. |
|
@jasmussen Here's another proposed top-level navigation item for global styles. I think we should map out how presets like shadow (and spacing eventually) should be presented in global styles. |
|
Agreed, the current IA is not scaling well. The issue feels a bit pressed by the too-granular "Background image". Going back to #47369, the main instinct there is to separate in two categories, presets and styles. Saxon suggests this:
"Shadows" is perhaps not the best term, but since it's a set of presets, the above heuristic would provide a structure for it at the top, alongside Colors and Typography. Layout is a bit tricky since it's in-between presets (through inheritance of block spacing and content widths), and styles for the current page (padding). I wonder if we could split that out, clean it up once and for all. The following is a quick idea intended to inspire better ideas:
"Background image" would go in "Site". As would "Padding", currently sitting in Layout. Block spacing would move from Layout to Spacing, along with other presets eventually. "Site" could also be a place where Border could be added. |
|
The separation makes sense, but unsure about "Styles" and "Presets" as headings. Would a shadow be a style or a preset... it's kind of both? The headings might not both be necessary, the labels are generally self-explanatory. Either way, probably a discussion for #53483. Saxon shared some interesting ideas there already. |
|
Would it work without subheadings? Maybe, though I think they make sense insofar as for shadows, you are only editing the presets there. You apply them (style your blocks) elsewhere. The subheadings also consider that "Blocks" is currently separated by both a line and introductory text. We can keep it that way. The thing is, we do need to make some progress for 6.6, especially for "Background image" which is so far removed from its context. #53483 is probably parallel but less likely to make it for 6.6. Unless we have better ideas, we should move "Background image" under "Layout". |
|
I guess it tripped me up because shadows can be both a style, and a style preset. Maybe I'm too used to the 'styles' concept in Figma. "Background" and "Layout" being grouped together under "Site" seems worth a try. |
|
Hi @madhusudhand, They are not shown as one of the available options to pick: There is also a small issue where if the shadow is too big it may make it impossible to correctly preview the other shadows. For colors, we show the multiple origins of colors: Are there any plans to allow to pick a specific shadow in a given place besides the preset ones? |
packages/edit-site/src/components/global-styles/shadows-edit-panel.js
Outdated
Show resolved
Hide resolved
|
Not for this PR but, Would it make sense when picking a color for the shadow show the preset color palettes? If a preset color is selected the shadow would reference it by its variable making the shadows dynamically change when the colors change. |
packages/edit-site/src/components/global-styles/shadows-panel.js
Outdated
Show resolved
Hide resolved
packages/edit-site/src/components/global-styles/shadows-panel.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Hi @madhusudhand, the main functionality is working well, and the code looks good 👍 Any specific part where a deeper look may be helpful?
madhusudhand
force-pushed
the
shadows-editor
branch
from
0cfc4bc
to
44a72d0
Compare
|
I am sorry, I missed copying props. |
* add shadows panel in global styles * add shadows editor view * add color edit popover * refine ui between default and custom shadows * update shadows UI to match new designs * add unit tests * Try different approach to parsing shadow strings * add more unit tests and improve util functions * update shadows edit panel * fix unit conversion issues and other ui improvements * add shadow rename and delete functions * address design feedback * add option to reset default and theme shadows * add custom shadows in block styles * remove local state and use momoize * fix lint issue * move reset option from shadows panel to shadow edit panel * split shadow dropdown button into two buttons * fix item height to 40px * validate invalid shadow strings * update spacing * delete comments --------- Co-authored-by: Aaron Robertshaw <[email protected]>
* add shadows panel in global styles * add shadows editor view * add color edit popover * refine ui between default and custom shadows * update shadows UI to match new designs * add unit tests * Try different approach to parsing shadow strings * add more unit tests and improve util functions * update shadows edit panel * fix unit conversion issues and other ui improvements * add shadow rename and delete functions * address design feedback * add option to reset default and theme shadows * add custom shadows in block styles * remove local state and use momoize * fix lint issue * move reset option from shadows panel to shadow edit panel * split shadow dropdown button into two buttons * fix item height to 40px * validate invalid shadow strings * update spacing * delete comments --------- Co-authored-by: Aaron Robertshaw <[email protected]>
What?
This change introduces a new item
shadowsin the global styles for editing/creating shadowsTesting instructions
shadowsin global styles panelcore,themeandcustomshadows.themeorcustomshadows, it should show a placeholder label.Editing a shadow:
Drop shadowto invoke editor, change lengths, color, inset etc.. and preview should update instantly.+to add new shadow layer.-to remove a shadow layer. (this is available when there are at least 2 layers)Subtasks
default,themeandcustomshadows.Related issues:
fixes: #57100