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

[Mobile] Fix title not focusing #15069

Merged
merged 2 commits into from
Apr 19, 2019
Merged

[Mobile] Fix title not focusing #15069

merged 2 commits into from
Apr 19, 2019

Conversation

etoledom
Copy link
Contributor

@etoledom etoledom commented Apr 19, 2019

This PR fixes an issue where the post title is not being focused when the user tap on it.
Related gutenberg-mobile PR: wordpress-mobile/gutenberg-mobile#893

I noticed that the:

return {
    clientId: context.clientId,
    isSelected: context.isSelected,
    onFocus: context.onFocus,
};

Is necessary for the ListBlock to get focus properly, but the addition of it broke the PostTitle, since context.onFocus is undefined when setting up the PostTitle's RichText component.

To solve it, we pass the onFocus from ownProps directly if context.onFocus is undefined.

I'd prefer to give context.onFocus preference, since it's how it is supposed to be set up for blocks, being the PostTitle the special case (the one that needs ownProps.onFocus).

The difference with Web is that they don't use RichText for the PostTitle.

To test:

  • Run the project.
  • Focus any block.
  • Focus on the PostTitle.
  • Check that it is focused with highlighted borders.
  • Create a ListBlock.
  • Focus other block and focus back the ListBlock.
  • Check that block selection works properly.

@etoledom etoledom 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 Apr 19, 2019
@etoledom etoledom self-assigned this Apr 19, 2019
@etoledom etoledom requested a review from hypest April 19, 2019 08:31
@etoledom etoledom marked this pull request as ready for review April 19, 2019 08:34
Copy link
Contributor

@hypest hypest left a comment

Choose a reason for hiding this comment

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

LGTM!

@etoledom etoledom merged commit 0883192 into master Apr 19, 2019
@etoledom
Copy link
Contributor Author

Thank you!

@etoledom etoledom deleted the rnmobile/fix-title-no-focus branch April 19, 2019 10:44
daniloercoli added a commit that referenced this pull request Apr 23, 2019
…rnmobile/887-History-stack-is-not-empty-on-a-fresh-start-of-the-editor

* 'master' of https://github.com/WordPress/gutenberg:
  'string' misspelled as 'srting' (#15118)
  [Mobile] Improving accessibility on Post title (#15106)
  Fix 13776: Format is already registered to handle class name (#15072)
  Add wpDataSelect WordPress end 2 end test util (#15052)
  Block Editor: move selection state from RichText to the store (#14640)
  chore: Fix: Lint error that makes unit tests (and CI tests) fail. (#15073)
  Set ownProps.onFocus when context.onFocus is undefined. (#15069)
@youknowriad youknowriad added this to the 5.6 (Gutenberg) milestone May 13, 2019
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