Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RNMobile] Update color retrieval from settings to useEditorFeature #25519

Merged

Conversation

cameronvoell
Copy link
Member

@cameronvoell cameronvoell commented Sep 21, 2020

GB-Mobile PR: wordpress-mobile/gutenberg-mobile#2648

Description

Fixes mobile crash on master after default color settings were updated here: #25419

How has this been tested?

We should test flows using default colors/gradients

We also need to make sure that custom theme still work which are set here:

this.props.updateSettings( theme );

Screenshots

Screenshot_20200921-134023_Gutenberg

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@cameronvoell cameronvoell added the Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) label Sep 21, 2020
@github-actions
Copy link

Size Change: 0 B

Total Size: 1.2 MB

ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.52 kB 0 B
build/api-fetch/index.js 3.34 kB 0 B
build/autop/index.js 2.72 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 8.41 kB 0 B
build/block-directory/style-rtl.css 943 B 0 B
build/block-directory/style.css 942 B 0 B
build/block-editor/index.js 128 kB 0 B
build/block-editor/style-rtl.css 11.1 kB 0 B
build/block-editor/style.css 11.1 kB 0 B
build/block-library/editor-rtl.css 8.59 kB 0 B
build/block-library/editor.css 8.59 kB 0 B
build/block-library/index.js 135 kB 0 B
build/block-library/style-rtl.css 7.6 kB 0 B
build/block-library/style.css 7.59 kB 0 B
build/block-library/theme-rtl.css 741 B 0 B
build/block-library/theme.css 741 B 0 B
build/block-serialization-default-parser/index.js 1.77 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 47.5 kB 0 B
build/components/index.js 202 kB 0 B
build/components/style-rtl.css 15.5 kB 0 B
build/components/style.css 15.5 kB 0 B
build/compose/index.js 9.42 kB 0 B
build/core-data/index.js 12 kB 0 B
build/data-controls/index.js 1.27 kB 0 B
build/data/index.js 8.43 kB 0 B
build/date/index.js 31.9 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 568 B 0 B
build/dom/index.js 4.42 kB 0 B
build/edit-navigation/index.js 10.4 kB 0 B
build/edit-navigation/style-rtl.css 868 B 0 B
build/edit-navigation/style.css 871 B 0 B
build/edit-post/index.js 306 kB 0 B
build/edit-post/style-rtl.css 6.24 kB 0 B
build/edit-post/style.css 6.22 kB 0 B
build/edit-site/index.js 19.6 kB 0 B
build/edit-site/style-rtl.css 3.3 kB 0 B
build/edit-site/style.css 3.3 kB 0 B
build/edit-widgets/index.js 17.6 kB 0 B
build/edit-widgets/style-rtl.css 2.79 kB 0 B
build/edit-widgets/style.css 2.79 kB 0 B
build/editor/editor-styles-rtl.css 492 B 0 B
build/editor/editor-styles.css 493 B 0 B
build/editor/index.js 45.3 kB 0 B
build/editor/style-rtl.css 3.8 kB 0 B
build/editor/style.css 3.8 kB 0 B
build/element/index.js 4.45 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.49 kB 0 B
build/format-library/style-rtl.css 547 B 0 B
build/format-library/style.css 548 B 0 B
build/hooks/index.js 1.74 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.54 kB 0 B
build/is-shallow-equal/index.js 711 B 0 B
build/keyboard-shortcuts/index.js 2.39 kB 0 B
build/keycodes/index.js 1.85 kB 0 B
build/list-reusable-blocks/index.js 3.02 kB 0 B
build/list-reusable-blocks/style-rtl.css 476 B 0 B
build/list-reusable-blocks/style.css 476 B 0 B
build/media-utils/index.js 5.12 kB 0 B
build/notices/index.js 1.69 kB 0 B
build/nux/index.js 3.27 kB 0 B
build/nux/style-rtl.css 671 B 0 B
build/nux/style.css 668 B 0 B
build/plugins/index.js 2.44 kB 0 B
build/primitives/index.js 1.34 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/rich-text/index.js 13.7 kB 0 B
build/server-side-render/index.js 2.6 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.24 kB 0 B
build/url/index.js 4.06 kB 0 B
build/viewport/index.js 1.74 kB 0 B
build/warning/index.js 1.13 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

@enejb
Copy link
Contributor

enejb commented Sep 21, 2020

Should we also add the removed default colors and gradients to block-editor/src/store/default.native.js ?

@cameronvoell cameronvoell marked this pull request as ready for review September 21, 2020 21:01
Copy link
Contributor

@chipsnyder chipsnyder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ran through the regression steps for editor theme

  • ✖️Default Colors - Check that default colors still load - steps
  • ✖️Default Gradients - Check that default gradients still load - steps
  • ✅ Custom Colors - Check that custom colors load in the editor - steps
  • ✅ Custom Gradients - Check that custom gradients load in the editor - steps
    - When changing a theme if the new theme downloads before the existing one then the editor needs to be reopened before it is reflected. This is to be a known issue.
  • ✅ Offline Support - steps

Overall this looks good and removes the crash. I think we can handle the top two test cases in different PR so we unblock current development.

@chipsnyder
Copy link
Contributor

Should we also add the removed default colors and gradients to block-editor/src/store/default.native.js ?

This is a good idea @cameronvoell I'll leave it up to you if you want to do that now or if you think that's better for another PR

Copy link
Contributor

@guarani guarani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested and can confirm no crash anymore, had a quick play with colors on the Cover and Button blocks and could pick the gradient colors only, no built-in choices as we discussed offline 👍

@cameronvoell cameronvoell changed the title Update color retrieval from settings to useEditorFeature [RNMobile] Update color retrieval from settings to useEditorFeature Sep 21, 2020
@cameronvoell cameronvoell merged commit 721ebaa into master Sep 21, 2020
@cameronvoell cameronvoell deleted the rnmobile/fix-color-settings-red-screen-cover-button branch September 21, 2020 21:32
@github-actions github-actions bot added this to the Gutenberg 9.1 milestone Sep 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants