Cover: Move overlay and opacity controls to color panel #41102
Merged
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
aaronrobertshaw
added
[Type] Enhancement
|
Size Change: -8 B (0%) Total Size: 1.24 MB
ℹ️ View Unchanged
|
aaronrobertshaw
force-pushed
the
refactor/cover-color-controls
branch
from
700c423 to
c518231
Compare
aaronrobertshaw
changed the base branch from
trunk
to
update/refactor-panel-color-gradient-settings
|
I've rebased this onto #41091 and tweaked accordingly. It should be right to review as a follow-up once #41091 lands. |
jorgefilipecosta
force-pushed
the
update/refactor-panel-color-gradient-settings
branch
from
e1ec461 to
ed7c83c
Compare
Base automatically changed from
update/refactor-panel-color-gradient-settings
to
trunk
May 18, 2022 18:23
aaronrobertshaw
force-pushed
the
refactor/cover-color-controls
branch
from
c518231 to
6f11b3d
Compare
|
With the design in Would be good to get a second opinion from a designer on this but should we move the the opacity slider to within the colour picker which is where it is in other blocks? Kapture.2022-05-20.at.11.28.52.mp4 |
|
Thanks for the review @noisysocks 👍
Agreed. The reasoning for injecting this into the Color supports panel was that there are also plans to add text color support to the Cover block which would result in two color related panels in the sidebar. Also, on trunk the panel's reset all menu item doesn't reset the cover block.
This is on my list to explore. The color picker allows transparency via the |
That makes sense. It think it makes it even more important that the slider moves into the colour picker, then, because otherwise it won't be clear if the slider adjusts the text opacity or overlay opacity. |
There was a problem hiding this comment.
Hi @noisysocks what if we name "Opacity" as "Overlay Opacity" to make it clear it refers to the overlay?
There are some reasons to have opacity as a separate control instead of as the transparency value of the color:
- We can easily set a default opacity as we do now.
- If the is a complex preset gradient with 6 color stops, and we want to make that gradient have 20% opacity it is easier to just select the opacity in a single place, than changing the 6 color stops to achieve the 20% opacity.
- If I want to use a preset color or gradient but just change the opacity, using separate opacities we only rely on classes we still use the preset classes if we go for the color transparency approach we would use an inline color.
aaronrobertshaw
force-pushed
the
refactor/cover-color-controls
branch
from
6f11b3d to
6b05ca5
Compare
Those are some great points. I think renaming the control is a nice, simple compromise. It also keeps this progressing forward to where we can explore opting into text color block support for the Cover block without having two color related panels. I've renamed the control from "Opacity" to "Overlay opacity" in 6b05ca5. |
|
Working great for me! There's one thing I noticed: I think we have to set the To replicate:
cover-with-controls.mp4I checked on trunk and I couldn't reproduce. |
|
Tests well for me, but I wonder if it is confusing to have the separate opacity option, and the alpha channel in the custom color picker: It is explicitly toggled on here, so maybe there is a reason for having both that I am overlooking? |
|
Thanks for raising that issue @glendaviesnz 👍 I've removed the alpha channel from the overlay color picker (4667be1). It was an oversight of mine while copy and pasting in the color dropdown code.
|
22 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.
Related:
Depends on:
What?
Moves the Cover block's overlay color and opacity controls into the inspector controls sidebar's Color panel.
Why?
Makes the Cover block's color-related controls appear under the same panel most blocks' do for consistency.
How?
Renders the Cover block's color-related controls via the grouped inspector controls slot so they are rendered alongside any color block supports and under the Color panel in the sidebar.
Testing Instructions
Screenshots or screencast