Skip to content

[RNMobile] Refactor BlockToolbar follow-up PR#16906

Merged
Tug merged 4 commits into
masterfrom
refactor/block-toolbar-fu-fix
Aug 5, 2019
Merged

[RNMobile] Refactor BlockToolbar follow-up PR#16906
Tug merged 4 commits into
masterfrom
refactor/block-toolbar-fu-fix

Conversation

@Tug
Copy link
Copy Markdown
Contributor

@Tug Tug commented Aug 5, 2019

Description

This PR fixes some minor issues with #16677

How has this been tested?

Tested with wordpress-mobile/gutenberg-mobile#1247

Types of changes

UX changes:

  • Hide keyboard when switching mode
  • Do not force hide keyboard on android, as unselecting a block already handles it

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.

@Tug Tug 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 Aug 5, 2019
@Tug Tug added this to the Future milestone Aug 5, 2019
@Tug Tug requested a review from mchowning August 5, 2019 16:22
@Tug Tug self-assigned this Aug 5, 2019
@Tug Tug requested a review from hypest August 5, 2019 16:24
} ),
withDispatch( ( dispatch ) => {
const {
clearSelectedBlock,
Copy link
Copy Markdown
Contributor

@mchowning mchowning Aug 5, 2019

Choose a reason for hiding this comment

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

It looks like clearSelectedBlock from core/editor just calls through to core/block-editor with a deprecation warning that shows up as a Yellow Box warning. Sounds like we should just get this directly from core/block-editor instead of via core/editor.

Screen Shot 2019-08-05 at 12 55 51 PM

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks, updated 👍

Copy link
Copy Markdown
Contributor

@mchowning mchowning Aug 5, 2019

Choose a reason for hiding this comment

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

I think we also need to update header-toolbar: https://github.com/WordPress/gutenberg/pull/16906/files#diff-dbd7e14b9675d5446422c008453093bbR91. Sorry I didn't realize that before.

Copy link
Copy Markdown
Contributor

@mchowning mchowning left a comment

Choose a reason for hiding this comment

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

LGTM! :shipit:

@Tug Tug merged commit 54323e9 into master Aug 5, 2019
@Tug Tug deleted the refactor/block-toolbar-fu-fix branch August 5, 2019 18:23
@youknowriad youknowriad modified the milestones: Future, Gutenberg 6.3 Aug 9, 2019
gziolo pushed a commit that referenced this pull request Aug 29, 2019
* Keyboard is already hidden with clearSelectedBlock

* Hide Keyboard when switching mode

* require clearSelectedBlock from core/block-editor instead of core/editor

* require clearSelectedBlock from core/block-editor instead of core/editor
gziolo pushed a commit that referenced this pull request Aug 29, 2019
* Keyboard is already hidden with clearSelectedBlock

* Hide Keyboard when switching mode

* require clearSelectedBlock from core/block-editor instead of core/editor

* require clearSelectedBlock from core/block-editor instead of core/editor
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.

3 participants