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

Stories/af/9601 replacement variables suggestions speak #525

Merged

Conversation

afercia
Copy link
Contributor

@afercia afercia commented May 14, 2018

Summary

This PR can be summarized in the following changelog entry:

  • Snippet editor: Improve the replacement variable suggestions accessibility

Relevant technical choices:

  • given the way this component re-renders, things are a bit more complicated than expected
  • when typing % to search for a replacement variable, both onSearchChange and onChange run the latter calls serializeContent which I guess fully re-renders the component because at that point the number of replacementVariables in the state is the initial ones from the props /Cc @atimmer
  • basically it is not possible to use componentDidUpdate()
  • there's the need to get the count of the filtered replacement variables after the state is updated and before the re-renders happen (which updates the state again, resetting it)
  • not ideal, but haven't found other ways to do it, any feedback welcome

Test instructions

  • run the standalone example (yarn start then go to http://localhost:3333/)
  • in the title and description fields, search for the replacement variables typing % etc.
  • observe in the DOM the div with id #a11y-speak-assertive gets a message with the correct number of search results
  • verify singular/plural forms (1 result, 2 results...)
  • verify there's a correct message when there are no results

Fixes Yoast/wordpress-seo#9601

@afercia afercia added this to the Snippet Preview milestone May 14, 2018
@afercia afercia changed the base branch from features/snippet-preview to develop May 14, 2018 11:23
@boblinthorst
Copy link
Contributor

CR: ok 👍
Acceptance: ok 👍

@boblinthorst boblinthorst modified the milestones: Snippet Preview, 3.6 May 14, 2018
@boblinthorst boblinthorst merged commit c005367 into develop May 14, 2018
@boblinthorst boblinthorst deleted the stories/af/9601-replacement-variables-suggestions-speak branch May 14, 2018 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants