Skip to content

Fix plain text selection/focus#19964

Merged
SergioEstevao merged 4 commits intomasterfrom
rnmobile/fix_plain_text_selection
Feb 5, 2020
Merged

Fix plain text selection/focus#19964
SergioEstevao merged 4 commits intomasterfrom
rnmobile/fix_plain_text_selection

Conversation

@SergioEstevao
Copy link
Copy Markdown
Contributor

@SergioEstevao SergioEstevao commented Jan 30, 2020

Description

I change the PlainText component code in order to properly focus when selected on mount.

How has this been tested?

Screenshots

Types of changes

Bug fix

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. .

marecar3 and others added 2 commits January 29, 2020 13:20
@marecar3
Upgrade react native to 0.62.0-rc.1
022d763
@SergioEstevao
Fix focus on plain text for RN
e0cc5e4 
It looks there was a type and we were checking the incorrect props.
@SergioEstevao SergioEstevao added the [Type] Bug An existing feature does not function as intended label Jan 30, 2020
@SergioEstevao SergioEstevao added this to the Future milestone Jan 30, 2020
etoledom
etoledom approved these changes Feb 5, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@etoledom etoledom left a comment

Choose a reason for hiding this comment

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

This was working before because we are passing all props given to PlainText down to TextInput, so the _input instance also has a isSelected prop.

Said that, checking agains the "original" this.props.isSelected makes more sense and is easier to reason about, and from the conversation with @SergioEstevao , this also solves an issue on the RN 0.62 update branch.

Let's :shipit: !

Thank you!

@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 Feb 5, 2020
@SergioEstevao
Merge branch 'master' into rnmobile/fix_plain_text_selection
164129f 
# Conflicts:
#	packages/block-editor/src/components/plain-text/index.native.js
@SergioEstevao SergioEstevao merged commit 818eb80 into master Feb 5, 2020
@SergioEstevao SergioEstevao deleted the rnmobile/fix_plain_text_selection branch February 5, 2020 23:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@etoledom etoledom etoledom approved these changes

Assignees

No one assigned

Labels

Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) [Type] Bug An existing feature does not function as intended

Projects

None yet

Milestone

Future

Development

Successfully merging this pull request may close these issues.

3 participants

@SergioEstevao @etoledom @marecar3