[Mobile] - Fix regression with the Color hook and ColorPanel #49917
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
geriux
added
[Status] In Progress
|
Size Change: -2.41 kB (0%) Total Size: 1.37 MB
ℹ️ View Unchanged
|
added 8 commits
April 19, 2023 16:20
…pdate code to latest changes from the color hook
…s since this is now being handled by the ColorPanel component
…nt data due to the latest color hook changes
geriux
force-pushed
the
rnmobile/fix/color-customization-for-blocks
branch
from
c1f4f77
to
29cad05
Compare
added 2 commits
April 19, 2023 17:24
… to the editor for tests
|
Flaky tests detected in 3994388. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4754875995
|
geriux
removed
the
Mobile App - Automation
4 tasks
dcalhoun
approved these changes
Apr 19, 2023
packages/block-editor/src/components/global-styles/color-panel.native.js
Show resolved
Hide resolved
| const buttonsBlock = getBlock( screen, 'Buttons' ); | ||
| fireEvent.press( buttonsBlock ); | ||
|
|
||
| // Trigger onLayout for the list |
There was a problem hiding this comment.
Given we have to do this so often, I question the value of placing this inline comment. Should we instead consider renaming the helper to more clearly communicate intent/rationale/outcome. WDYT?
Comment on lines
+294
to
+301
| // Open Block Settings. | ||
| fireEvent.press( screen.getByLabelText( 'Open Settings' ) ); | ||
|
|
||
| // Wait for Block Settings to be visible. | ||
| const blockSettingsModal = screen.getByTestId( | ||
| 'block-settings-modal' | ||
| ); | ||
| await waitFor( () => blockSettingsModal.props.isVisible ); |
There was a problem hiding this comment.
There appears to be an openBlockSettings helper we should considering using.
Comment on lines
+309
to
+310
| // Dismiss the Block Settings modal. | ||
| fireEvent( blockSettingsModal, 'backdropPress' ); |
There was a problem hiding this comment.
There appears to be a dismissModal helper we should consider using.
packages/components/src/mobile/color-settings/palette.screen.native.js
Outdated
Show resolved
Hide resolved
dcalhoun
reviewed
Apr 20, 2023
added 2 commits
April 20, 2023 15:25
…ntAST when calling getGradientAstWithDefault
…ents, in some cases these values can be undefined, depending on the theme colors/configuration. It also adds a fallback name for colors that don't have a name value
Thank you for testing and reviewing! There are a few comments related to tests that I'd like to address them in another PR so we can move this forward. I still need to fix a visual regression test but let me know if there are other things to check out before merging (with the new changes I added today) Thanks again! |
dcalhoun
approved these changes
Apr 20, 2023
geriux
pushed a commit
that referenced
this pull request
Apr 20, 2023
* Mobile - Update ColorPanel: Move it to the global styles folder and update code to latest changes from the color hook * Mobile - ColorSettings Palette: Avoid resetting color/gradient changes since this is now being handled by the ColorPanel component * Mobile - CustomGradientPicker - Update code to get the correct gradient data due to the latest color hook changes * Mobile - ColorPalette - Add accessibilityLabel with the color value * Mobile - Segmented Control - Add optional chaining * Mobile - Paragraph block - Add tests for color customization * Mobile - Heading block - Add tests for color customization * Mobile - Buttons block - Add tests for color customization * Mobile - Integration tests - Add getGlobalStyles - To pass theme data to the editor for tests * Mobile - Paragraph block - Test setting a theme text color and for the contrast check * Mobile - ColorPalette - Use color name as the accessibility label * Mobile - CustomGradientPicker - Simplify code by destructuring gradientAST when calling getGradientAstWithDefault * Mobile - ColorPalette - Add optional chaining for allColors and gradients, in some cases these values can be undefined, depending on the theme colors/configuration. It also adds a fallback name for colors that don't have a name value
4 tasks
geriux
pushed a commit
that referenced
this pull request
Apr 21, 2023
* Release script: Update react-native-editor version to 1.93.0 * Release script: Update with changes from 'npm run core preios' * Update Changelog * Release script: Update react-native-editor version to 1.93.1 * Release script: Update with changes from 'npm run core preios' * test: Expand multi-line block tests (#49732) * test: Fix typo in Preformatted test * test: Rich Text helpers can append text The addition of `initialSelectionStart` and `initialSelectEnd` enable placing the caret prior to "typing" additional text. This API was inspired by the `testing-library/user-event` APIs. https://testing-library.com/docs/user-event/utility#type * test: Expand multi-line components typing tests Tests continue typing after inserting a line break now that test helpers support that ability. * test: Fix List block split and merge tests Changes to the `onChangeAndSelectText` helper means text is appended by default. To recreate merely placing the cursor in the beginning of the text, we must re-type the text as the `onChangeAndSelectText` changes the text in addition to selecting due to the selection logic invoking change logic within `RichText`. * test: Rich text test helpers mimic user events (#49804) * test: changeAndSelectTextOfRichText defaults cursor to end of new text This will mimic natural typing where the cursor is placed at the end of newly typed text by default. * test: Consolidate multiple Rich Text change text helpers The existing helpers contained substantial overlap and were coupled to implementation details of the `RichText` event handlers. This refactor creates a new, single helper to mimic typing into rich text fields as a user might. * test: Add selectTextInRichText helper Simply process for selecting rich text values to improve test comprehensibility. * test: Simplify tests with new helpers It is no longer necessary to pass the length of the test strings as the new helper automatically tracks the string length to place the cursor at the end of the new value, mimicking a user typing. * test: Refactor selectTextInRichText Rename helper and require range start position to improve clarity of the purpose and function of the now `selectRangeInRichText` helper. * test: Rename typeInRichText selection parameters Improve clarity by further differentiating the initial and final selection range. * [Mobile] - Fix regression with the Color hook and ColorPanel (#49917) * Mobile - Update ColorPanel: Move it to the global styles folder and update code to latest changes from the color hook * Mobile - ColorSettings Palette: Avoid resetting color/gradient changes since this is now being handled by the ColorPanel component * Mobile - CustomGradientPicker - Update code to get the correct gradient data due to the latest color hook changes * Mobile - ColorPalette - Add accessibilityLabel with the color value * Mobile - Segmented Control - Add optional chaining * Mobile - Paragraph block - Add tests for color customization * Mobile - Heading block - Add tests for color customization * Mobile - Buttons block - Add tests for color customization * Mobile - Integration tests - Add getGlobalStyles - To pass theme data to the editor for tests * Mobile - Paragraph block - Test setting a theme text color and for the contrast check * Mobile - ColorPalette - Use color name as the accessibility label * Mobile - CustomGradientPicker - Simplify code by destructuring gradientAST when calling getGradientAstWithDefault * Mobile - ColorPalette - Add optional chaining for allColors and gradients, in some cases these values can be undefined, depending on the theme colors/configuration. It also adds a fallback name for colors that don't have a name value * Update Changelog --------- Co-authored-by: David Calhoun <github@davidcalhoun.me>
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 PRs:
What?
This PR is a follow-up of #48893 that refactored the usage of the
Colorhook and theColorPanelcomponent. These changes broke the Text and Background color customizations in the mobile editor.Why?
To bring back the Color and Background color customization functionality to the mobile editor.
How?
It moves the native
ColorPanelto thecomponents/global-stylesto keep the same structure as the Web editor, it brings the web changes and keeps the current functionality we had for the ContrastChecker.Removes current code like avoiding manually resetting color changes since it will be managed by the ColorPanel.
Adds new
getGlobalStylesintegration helper that can be used withinitializeEditorpassingwithGlobalStylesastruewhich will pass to the editor some Global styles data. This can be expanded in the future depending on the requirements.It also adds the following tests to prevent future breakages related to colors customizations:
Paragraph block:
Heading block:
Buttons block:
Testing Instructions
Testing Instructions for Keyboard
N/A
Screenshots or screencast
ColorTests.mov