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

Fix @-mentions UI to allow @ character in posts #14596

Closed
wants to merge 1 commit into from

Conversation

guarani
Copy link
Contributor

@guarani guarani commented Aug 6, 2020

Addresses WordPress/gutenberg#24425
Related WordPress/gutenberg#24428
Related wordpress-mobile/gutenberg-mobile#2536

To test: See WordPress/gutenberg#24428 (comment)

PR submission checklist:

  • I have considered adding unit tests where possible.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Aug 6, 2020

You can test the changes on this Pull Request by downloading it from AppCenter here with build number: 32609. IPA is available here. If you need access to this, you can ask a maintainer to add you.

This fixes a bug with the @-mention feature that made entering the standalone @ character difficult because of the @-mention UI getting in the way.
@guarani guarani changed the base branch from develop to release/15.4 August 6, 2020 23:29
@peril-wordpress-mobile
Copy link

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

@hypest
Copy link
Contributor

hypest commented Aug 7, 2020

I tried build 32609 from Appcenter:

  1. Typed a "@" at the start of a block, mentions UI popped up, still in "loading" state since it was from a fresh install, I tapped outside of the UI while still "loading" and the UI dismissed and the "@" was there in the block ✅
  2. Typed a "@" at the start of a block, mentions UI popped up filled with many items, typed backspace and the UI dismissed leaving a "@" in the block ✅
  3. Triggered the mentions UI, typed a few more chars after the "@" while in "loading" state, tapped outside to dismiss and it was only a "@" left in the block ✅
  4. Triggered the UI via the toolbar button, tapped on the empty space while the UI was in "loading" state and returned to the block without the "@" being there ✅
  5. Triggered the UI via the toolbar button, backspaced to delete the "@" and the UI dismissed without the "@" staying in the block ✅

So, functionally this looks good to me!

I'll have a look at the diff as well.
Edit: in the current state, the PR brings in more changes from the editor release that we don't want to ship to 15.4 yet. So, this PR will need updating to use a hotfix version (1.31.1) on the editor side.

@hypest
Copy link
Contributor

hypest commented Aug 7, 2020

Headsup that @ceyhun will be adding effort directly on this fix PRs so we make it on time for the apps releases.

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.

Approved via WordPress/gutenberg#24428 (review)

Tested after the latest update (to merge to the release branch).

@guarani guarani added this to the 15.4 ❄️ milestone Aug 7, 2020
@ceyhun
Copy link
Contributor

ceyhun commented Aug 7, 2020

Closing in favor of #14599

@ceyhun ceyhun closed this Aug 7, 2020
@ceyhun ceyhun deleted the gutenberg/fix/@-chars-in-posts branch August 7, 2020 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants