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

React Native: Fix @-mentions UI to allow @ character in posts #24428

Merged
merged 1 commit into from
Aug 7, 2020

Conversation

guarani
Copy link
Contributor

@guarani guarani commented Aug 6, 2020

Description

Addresses #24425

This address a bug with the @-mention feature that made entering the standalone @ character difficult because of the @-mention UI getting in the way.
The fix here is to allow the @ character to be intercepted without being consumed by the key event logic. This addresses one aspect of proposed fix for the issue linked above:

tap backspace after adding a @ character, which should dismiss the mentions UI and leave the @ character in the post

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.

@github-actions
Copy link

github-actions bot commented Aug 6, 2020

Size Change: -1.03 MB (89%) 🏆

Total Size: 1.15 MB

Filename Size Change
build/a11y/index.js 0 B -1.14 kB (0%)
build/annotations/index.js 0 B -3.67 kB (0%)
build/api-fetch/index.js 0 B -3.44 kB (0%)
build/autop/index.js 0 B -2.82 kB (0%)
build/blob/index.js 0 B -620 B (0%)
build/block-directory/index.js 0 B -7.97 kB (0%)
build/block-directory/style-rtl.css 944 B -9 B (0%)
build/block-directory/style.css 945 B -7 B (0%)
build/block-editor/index.js 0 B -125 kB (0%)
build/block-editor/style-rtl.css 10.8 kB +205 B (1%)
build/block-editor/style.css 10.8 kB +201 B (1%)
build/block-library/editor-rtl.css 7.58 kB -13 B (0%)
build/block-library/editor.css 7.57 kB -16 B (0%)
build/block-library/index.js 0 B -132 kB (0%)
build/block-library/style-rtl.css 7.77 kB +3 B (0%)
build/block-library/style.css 7.77 kB +3 B (0%)
build/block-library/theme-rtl.css 728 B -1 B
build/block-library/theme.css 729 B -1 B
build/block-serialization-default-parser/index.js 0 B -1.88 kB (0%)
build/block-serialization-spec-parser/index.js 0 B -3.1 kB (0%)
build/blocks/index.js 0 B -48.3 kB (0%)
build/components/index.js 0 B -200 kB (0%)
build/components/style-rtl.css 15.6 kB -62 B (0%)
build/components/style.css 15.6 kB -61 B (0%)
build/compose/index.js 0 B -9.68 kB (0%)
build/core-data/index.js 0 B -11.8 kB (0%)
build/data-controls/index.js 0 B -1.29 kB (0%)
build/data/index.js 0 B -8.45 kB (0%)
build/date/index.js 0 B -5.38 kB (0%)
build/deprecated/index.js 0 B -772 B (0%)
build/dom-ready/index.js 0 B -568 B (0%)
build/dom/index.js 0 B -3.23 kB (0%)
build/edit-navigation/index.js 0 B -10.9 kB (0%)
build/edit-post/index.js 0 B -304 kB (0%)
build/edit-post/style-rtl.css 5.61 kB +17 B (0%)
build/edit-post/style.css 5.61 kB +17 B (0%)
build/edit-site/index.js 0 B -17 kB (0%)
build/edit-widgets/index.js 0 B -9.38 kB (0%)
build/editor/index.js 0 B -45.3 kB (0%)
build/editor/style-rtl.css 3.78 kB -17 B (0%)
build/editor/style.css 3.78 kB -16 B (0%)
build/element/index.js 0 B -4.65 kB (0%)
build/escape-html/index.js 0 B -733 B (0%)
build/format-library/index.js 0 B -7.72 kB (0%)
build/hooks/index.js 0 B -2.13 kB (0%)
build/html-entities/index.js 0 B -621 B (0%)
build/i18n/index.js 0 B -3.56 kB (0%)
build/is-shallow-equal/index.js 0 B -711 B (0%)
build/keyboard-shortcuts/index.js 0 B -2.52 kB (0%)
build/keycodes/index.js 0 B -1.94 kB (0%)
build/list-reusable-blocks/index.js 0 B -3.11 kB (0%)
build/media-utils/index.js 0 B -5.33 kB (0%)
build/notices/index.js 0 B -1.79 kB (0%)
build/nux/index.js 0 B -3.4 kB (0%)
build/plugins/index.js 0 B -2.56 kB (0%)
build/primitives/index.js 0 B -1.41 kB (0%)
build/priority-queue/index.js 0 B -789 B (0%)
build/redux-routine/index.js 0 B -2.85 kB (0%)
build/rich-text/index.js 0 B -13.9 kB (0%)
build/server-side-render/index.js 0 B -2.71 kB (0%)
build/shortcode/index.js 0 B -1.7 kB (0%)
build/token-list/index.js 0 B -1.27 kB (0%)
build/url/index.js 0 B -4.06 kB (0%)
build/viewport/index.js 0 B -1.85 kB (0%)
build/warning/index.js 0 B -1.14 kB (0%)
build/wordcount/index.js 0 B -1.17 kB (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.min.js 1.14 kB 0 B
build/annotations/index.min.js 3.67 kB 0 B
build/api-fetch/index.min.js 3.43 kB 0 B
build/autop/index.min.js 2.82 kB 0 B
build/blob/index.min.js 620 B 0 B
build/block-directory/index.min.js 7.63 kB 0 B
build/block-editor/index.min.js 125 kB 0 B
build/block-library/index.min.js 132 kB 0 B
build/block-serialization-default-parser/index.min.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.min.js 3.1 kB 0 B
build/blocks/index.min.js 48.3 kB 0 B
build/components/index.min.js 200 kB 0 B
build/compose/index.min.js 9.67 kB 0 B
build/core-data/index.min.js 11.5 kB 0 B
build/data-controls/index.min.js 1.29 kB 0 B
build/data/index.min.js 8.45 kB 0 B
build/date/index.min.js 5.38 kB 0 B
build/deprecated/index.min.js 772 B 0 B
build/dom-ready/index.min.js 568 B 0 B
build/dom/index.min.js 3.23 kB 0 B
build/edit-navigation/index.min.js 10.8 kB 0 B
build/edit-navigation/style-rtl.css 1.08 kB 0 B
build/edit-navigation/style.css 1.08 kB 0 B
build/edit-post/index.min.js 304 kB 0 B
build/edit-site/index.min.js 16.8 kB 0 B
build/edit-site/style-rtl.css 3.06 kB 0 B
build/edit-site/style.css 3.06 kB 0 B
build/edit-widgets/index.min.js 9.35 kB 0 B
build/edit-widgets/style-rtl.css 2.45 kB 0 B
build/edit-widgets/style.css 2.45 kB 0 B
build/editor/editor-styles-rtl.css 537 B 0 B
build/editor/editor-styles.css 539 B 0 B
build/editor/index.min.js 45.2 kB 0 B
build/element/index.min.js 4.65 kB 0 B
build/escape-html/index.min.js 733 B 0 B
build/format-library/index.min.js 7.72 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.min.js 2.13 kB 0 B
build/html-entities/index.min.js 621 B 0 B
build/i18n/index.min.js 3.56 kB 0 B
build/is-shallow-equal/index.min.js 711 B 0 B
build/keyboard-shortcuts/index.min.js 2.51 kB 0 B
build/keycodes/index.min.js 1.94 kB 0 B
build/list-reusable-blocks/index.min.js 3.12 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.min.js 5.32 kB 0 B
build/notices/index.min.js 1.79 kB 0 B
build/nux/index.min.js 3.4 kB 0 B
build/nux/style-rtl.css 671 B 0 B
build/nux/style.css 668 B 0 B
build/plugins/index.min.js 2.56 kB 0 B
build/primitives/index.min.js 1.4 kB 0 B
build/priority-queue/index.min.js 789 B 0 B
build/redux-routine/index.min.js 2.85 kB 0 B
build/rich-text/index.min.js 13.9 kB 0 B
build/server-side-render/index.min.js 2.71 kB 0 B
build/shortcode/index.min.js 1.7 kB 0 B
build/token-list/index.min.js 1.27 kB 0 B
build/url/index.min.js 4.06 kB 0 B
build/viewport/index.min.js 1.85 kB 0 B
build/warning/index.min.js 1.14 kB 0 B
build/wordcount/index.min.js 1.17 kB 0 B

compressed-size-action

@guarani guarani added the [Type] Bug An existing feature does not function as intended label Aug 6, 2020
@guarani guarani changed the title React Native: @-mentions UI impeds use of @ character in posts React Native: Fix @-mentions UI to allow @ character in posts Aug 6, 2020
@guarani guarani marked this pull request as ready for review August 6, 2020 23:07
@guarani guarani requested a review from mchowning August 6, 2020 23:08
This fixes a bug with the @-mention feature that made entering the standalone @ character difficult because of the @-mention UI getting in the way.
The fix here is to allow the @ character to be intercepted without being consumed by the @ key event logic.
@ceyhun ceyhun force-pushed the rnmobile/allow-@-char-in-posts branch from 7237a57 to 43e35c8 Compare August 7, 2020 10:49
@ceyhun ceyhun changed the base branch from master to rnmobile/release_1.33.1 August 7, 2020 10:50
Copy link
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.

Tested via wordpress-mobile/WordPress-iOS#14596

Working great as described. Nice job @guarani ! 🎉

On a side note: I wonder about the reasoning behind choosing this particular solution. Personally I'd say that dismissing the UI adding a space is more natural without breaking the typing flow. That's also how web does it too. But probably having all the ways implemented is the best option anyway 👍

@guarani
Copy link
Contributor Author

guarani commented Aug 7, 2020

Thanks a lot for the review, @etoledom!

On a side note: I wonder about the reasoning behind choosing this particular solution. Personally I'd say that dismissing the UI adding a space is more natural without breaking the typing flow. That's also how web does it too. But probably having all the ways implemented is the best option anyway 👍

Supporting dismissing the UI via backspace brings iOS behavior in-line with Android. We do plan to support on both platforms in the future the "space to dismiss" approach described in the issue this addresses:

tap space after adding a @ character, which should dismiss the mentions UI and leave the @ character in the post

@etoledom
Copy link
Contributor

etoledom commented Aug 7, 2020

We do plan to support on both platforms in the future the "space to dismiss" approach described in the issue this addresses

Amazing, thank you! 🙏

@guarani guarani merged commit 08298cd into rnmobile/release_1.33.1 Aug 7, 2020
@guarani guarani deleted the rnmobile/allow-@-char-in-posts branch August 7, 2020 13:45
cameronvoell added a commit that referenced this pull request Aug 7, 2020
* Release script: Update react-native-editor version to 1.33.0

* Update release notes.

* Update version numbers for packages.

* Revert "Reduce spacing between label and slider control (#23580)" (#24109)

This reverts commit 2a67de0.

* Release script: Update react-native-* versions to 1.33.1

* Release script: Update with changes from 'npm run core preios'

* Allow @ char in posts in mobile editor (#24428)

This fixes a bug with the @-mention feature on mobile that made entering the standalone @ character difficult because of the @-mention UI getting in the way.
The fix here is to allow the @ character to be intercepted without being consumed by the @ key event logic.

* Remove duplicate entry from changelog

Co-authored-by: Sergio Estevao <sergioestevao@gmail.com>
Co-authored-by: Antonis Lilis <antonis.lilis@gmail.com>
Co-authored-by: Ceyhun Ozugur <ceyhunozugur@gmail.com>
Co-authored-by: Paul Von Schrottky <paul.von.schrottky@automattic.com>
cameronvoell added a commit that referenced this pull request Aug 7, 2020
* Release script: Update react-native-editor version to 1.33.0

* Update release notes.

* Update version numbers for packages.

* Revert "Reduce spacing between label and slider control (#23580)" (#24109)

This reverts commit 2a67de0.

* Release script: Update react-native-* versions to 1.33.1

* Release script: Update with changes from 'npm run core preios'

* Allow @ char in posts in mobile editor (#24428)

This fixes a bug with the @-mention feature on mobile that made entering the standalone @ character difficult because of the @-mention UI getting in the way.
The fix here is to allow the @ character to be intercepted without being consumed by the @ key event logic.

* Remove duplicate entry from changelog

Co-authored-by: Sergio Estevao <sergioestevao@gmail.com>
Co-authored-by: Antonis Lilis <antonis.lilis@gmail.com>
Co-authored-by: Ceyhun Ozugur <ceyhunozugur@gmail.com>
Co-authored-by: Paul Von Schrottky <paul.von.schrottky@automattic.com>
cameronvoell added a commit that referenced this pull request Aug 11, 2020
* Release script: Update react-native-editor version to 1.34.0

* [RNMobile] E2E Android - Use swipe gesture to scroll inserter menu (#24338)

* Set autosaveInterval to 1 on mobile (#24353)

* Revert "[RNMobile] Fix jumping toolbar (#23684)" (#24388)

This reverts commit d86cd5f.

* Turn off autosave interval for mobile (#24415)

* [RNMobile] Merge release 1.33.1 to master (#24448)

* Release script: Update react-native-editor version to 1.33.0

* Update release notes.

* Update version numbers for packages.

* Revert "Reduce spacing between label and slider control (#23580)" (#24109)

This reverts commit 2a67de0.

* Release script: Update react-native-* versions to 1.33.1

* Release script: Update with changes from 'npm run core preios'

* Allow @ char in posts in mobile editor (#24428)

This fixes a bug with the @-mention feature on mobile that made entering the standalone @ character difficult because of the @-mention UI getting in the way.
The fix here is to allow the @ character to be intercepted without being consumed by the @ key event logic.

* Remove duplicate entry from changelog

Co-authored-by: Sergio Estevao <sergioestevao@gmail.com>
Co-authored-by: Antonis Lilis <antonis.lilis@gmail.com>
Co-authored-by: Ceyhun Ozugur <ceyhunozugur@gmail.com>
Co-authored-by: Paul Von Schrottky <paul.von.schrottky@automattic.com>

* Update version numbers

* Ran pod install to update podfile.lock

Co-authored-by: Drapich Piotr <drapich.piotr@gmail.com>
Co-authored-by: Adam Zielinski <adam@adamziel.com>
Co-authored-by: Chip <chip.snyder3@gmail.com>
Co-authored-by: Cameron Voell <cameronvoell@gmail.com>
Co-authored-by: Sergio Estevao <sergioestevao@gmail.com>
Co-authored-by: Antonis Lilis <antonis.lilis@gmail.com>
Co-authored-by: Paul Von Schrottky <paul.von.schrottky@automattic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants