[RNMobile] Display custom color value in mobile Cover Block color picker #51414
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
derekblank
added
Mobile App - i.e. Android or iOS
|
Size Change: 0 B Total Size: 1.44 MB ℹ️ View Unchanged
|
|
Flaky tests detected in 096f33a. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5675132613
|
derekblank
changed the title
derekblank
changed the title
geriux
reviewed
Jul 20, 2023
| styles.selectedColorTextDark | ||
| ); | ||
|
|
||
| function getBottomLabelText() { |
There was a problem hiding this comment.
I was wondering if we could have a variable instead of a function, what do you think?
|
I've tested these changes and they're working well on both iOS and Android 🎉 Can we add an integration test for this new functionality? |
|
@dcalhoun If you have a moment to take a look, I'm curious if you have an insight into why this integration test for this component might be failing. When running the test for I would expect that this error is due to this stylesheet object not being mocked somewhere. I am not too familiar with mocking style objects like this, but noted this code block in the same test referencing mocked stylesheet objects: gutenberg/packages/block-library/src/cover/test/edit.native.js Lines 34 to 44 in 04c5075 Could they be related, perhaps? |
@derekblank I opened #52943 to fix the style failures and others identified when proceeding further. That PR includes inline comments explaining the fixes as well. Please feel free to merge, cherry pick, or disregard the commits. Hope this helps. |
* test: Mock required styles for Cover block tests JavaScript logic fails due to undefined references from mocked Sass files. * test: Fix Cover block HEX test assertions Synchronous queries of user-facing text is generally preferred over test IDs or asynchronous queries. Additionally, the `react-native-hsv-color-picker` must be mocked to trigger the required interaction events. Lastly, selecting the white color results in a non-custom color, which does not appear to display the HEX value in the picker footer label.
derekblank
force-pushed
the
rnmobile/display-value-in-colorpicker
branch
from
096f33a
to
81abcec
Compare
4 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.
What?
Displays the hex color value when selecting a custom color on the mobile editor. Fixes:
Gutenberg Mobile PR: wordpress-mobile/gutenberg-mobile#5852
Why?
For color contrast accessibility reasons, it is helpful to have the value of the color displayed in a text or number format other than the color itself. Also, displaying the hex value matches other Gutenberg color picker implementations on web and mobile (like the paragraph block, for example).
How?
When a custom color is selected in the Cover block, replace the "Select a color" label with the hex color value within the bottom sheet.
Testing Instructions
Screenshots or screencast
(Note: the video player controls may obscure the actual PR changes at bottom of this video. Ensure you are not hovering over the video in order to see the hex value.)
before.mov
After.mov