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

Dismiss suggestions on space keystroke #16770

Merged
merged 1 commit into from
Jul 9, 2021
Merged

Conversation

guarani
Copy link
Contributor

@guarani guarani commented Jun 29, 2021

Addresses WordPress/gutenberg#24425 (for iOS)

Note: This PR replaces #16601 which was reverted after it was found that the PR overlooked updating analytics events.

This PR allows users to easily dismiss the suggestion UI by typing a space character after triggering the UI. This means it will now be easier to add a @ character to a post without triggering the mentions suggestions UI, or add a + character without triggering the cross-post suggestions UI.

I added this change to suggestions in the block editor, but didn't apply it to other parts of the app (e.g. comments, notification replies, etc) because that felt too far out-of-scope here. It would also make the testing steps much longer.

To test:

- [ ] Verify user can dismiss @-mention suggestions on space keystroke
  - Type a space after a `@` character, expect the mentions suggestions list to be dismissed
  - Expect that the `wpios_suggestion_session_finished` Tracks event fires with the `did_select_suggestion` custom property set to `false`
- [ ] Verify user can dismiss cross-post suggestions on space keystroke  
  - Type a space after a `+` character, expect the cross-positing suggestions list to be dismissed
  - Expect that the `wpios_suggestion_session_finished` Tracks event fires with the `did_select_suggestion` custom property set to `false`
- [ ] Verify user can still @-mention another user
  - Expect the recipient of the @-mention to be notified
  - Expect that the `wpios_suggestion_session_finished` Tracks event fires with the `did_select_suggestion` custom property set to `true`
- [ ] Verify user can still cross-post to another site
  - Expect the recipient site to receive the cross-post
  - Expect the `wpios_suggestion_session_finished` Tracks event fires with the `did_select_suggestion` custom property set to `false`

Regression Notes

  1. Potential unintended areas of impact

This change could conceivably break suggestions (both mentions and cross-posting).

  1. What I did to test those areas of impact (or what existing automated tests I relied on)

The above manual tests.

  1. What automated tests I added (or what prevented me from doing so)

This change affects the UI so a UI test would be best, but we only add UI tests to core flows.

PR submission checklist:

  • I have completed the Regression Notes.
  • I have considered adding unit tests for my changes.
  • 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.

Users expect the suggestions UI to be dismissed when typing a space immediately after a trigger character (e.g. `@` for mentions, `+` for cross-posts).
@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Jun 29, 2021

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

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Jun 29, 2021

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

@guarani guarani added this to the 17.8 milestone Jun 29, 2021
@guarani
Copy link
Contributor Author

guarani commented Jun 29, 2021

Testing 7b74c84 on iPhone 11:

  • Verify user can dismiss @-mention suggestions on space keystroke
    • Type a space after a @ character, expect the mentions suggestions list to be dismissed
    • Expect that the wpios_suggestion_session_finished Tracks event fires with the did_select_suggestion custom property set to false
  • Verify user can dismiss cross-post suggestions on space keystroke
    • Type a space after a + character, expect the cross-positing suggestions list to be dismissed
    • Expect that the wpios_suggestion_session_finished Tracks event fires with the did_select_suggestion custom property set to false
  • Verify user can still @-mention another user
    • Expect the recipient of the @-mention to be notified
    • Expect that the wpios_suggestion_session_finished Tracks event fires with the did_select_suggestion custom property set to true
  • Verify user can still cross-post to another site
    • Expect the recipient site to receive the cross-post
    • Expect the wpios_suggestion_session_finished Tracks event fires with the did_select_suggestion custom property set to true

@guarani guarani requested a review from enejb June 29, 2021 01:57
@guarani
Copy link
Contributor Author

guarani commented Jun 29, 2021

Would you mind reviewing this @enejb when you get a chance? The only difference between this and #16601 (which you reviewed) is that this PR handles analytics properly.

@SiobhyB
Copy link
Contributor

SiobhyB commented Jul 1, 2021

@guarani, I just wanted to share that the Android side was approved and I (perhaps too hastily!) merged it today. Let me know if that seems okay to you or if you think it'd be best to revert the merge in order to make sure these are both aligned.

@guarani
Copy link
Contributor Author

guarani commented Jul 8, 2021

@guarani, I just wanted to share that the Android side was approved and I (perhaps too hastily!) merged it today. Let me know if that seems okay to you or if you think it'd be best to revert the merge in order to make sure these are both aligned.

Sorry @SiobhyB, I missed this ping. I'd say it's OK 👍 It shouldn't be a problem.

Copy link
Contributor

@enejb enejb left a comment

Choose a reason for hiding this comment

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

Nice work! This tested for me as advertised.

@guarani guarani merged commit 0f1a105 into develop Jul 9, 2021
@guarani guarani deleted the fix/suggestion-ui-dismissal branch July 9, 2021 19:39
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.

3 participants