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

[Localization] Support an optional object of Localized phrases for Screen Reader announcements #57

Open
1 of 3 tasks
inkblotty opened this issue Dec 10, 2021 · 12 comments

Comments

@inkblotty
Copy link
Contributor

inkblotty commented Dec 10, 2021

Context

Currently, all the screen reader announcements for this component are in English and have very generic wording such as "Results hidden". You may want to use more specific verbiage for your component use case, and/or support multiple languages.

Acceptance Criteria

  • Extend the autocomplete API to allow the developer to pass in localized phrases for every Screen Reader announcement
  • If one phrase isn't passed in, fallback to the default already defined (We've got a lot of feedback in the verbiage here.)
  • Create unit tests to ensure these phrases work correctly
@brunoprietog
Copy link
Contributor

Hello, how to move forward with this? Do you recommend any way on how to proceed? I can collaborate

@inkblotty
Copy link
Contributor Author

👋 Hey @brunoprietog -- Feel free to pick this up and assign yourself if you like. You can cc me and the other contributors on any PRs / drafts. Would be awesome to see this come together.

@brunoprietog
Copy link
Contributor

Great. Do you have any proposal of how this screen reader ads API could be? With data-something attributes or with divs that have some specific id.

@inkblotty
Copy link
Contributor Author

@brunoprietog:

The current screen-reader feedback div is identified as ${this.results_id}-feedback ID on this line.

I think the majority of the work would need to be done within this.updateFeedbackForScreenReaders to interpret the phrases based on some JSON blob maybe? We could pass it into the auto-complete-element using a stringified data attribute. We could see how that approach performs. What do you think @keithamus?

@keithamus
Copy link
Member

updateFeedbackForScreenReaders could emit an event with a mutable value which it then uses to read place into the feedback element. So for example:

class FeedbackEvent extends AutocompleteEvent {
  constructor(public text: string) {
    super('auto-complete-feedback', { bubbles: true, cancelable: true })
  }
}
  updateFeedbackForScreenReaders(inputString: string): void {
    const event = new FeedbackEvent(inputString)
    if (!this.dispatchEvent(event)) return
    setTimeout(() => {
      if (this.feedback) {
        this.feedback.textContent = event.text
      }
    }, SCREEN_READER_DELAY)
  }

This will then allow for consumers to override the default text:

el.addEventListener('auto-complete-feedback', (event) => {
  if (json.length === 0) {
    event.text = my_i18n('No users')
  } else {
    event.text = my_i18n(event.text)
  }
})

@inkblotty
Copy link
Contributor Author

inkblotty commented Aug 12, 2022

@keithamus - I know we have a delay on the screen reader feedback, but I'm concerned that triggering an event based on the feedback changing would announce the default phrase AND the localized phrase. We'd have to test carefully.

@keithamus
Copy link
Member

Events are synchronous, so as long as it is fired just before the announcement is triggered, then whatever phrase is in event.text will be read.

@inkblotty
Copy link
Contributor Author

@keithamus -- The aria-live attribute will ensure every change to the element's text is announced though. I would anticipate this won't be the right solution.

@keithamus
Copy link
Member

keithamus commented Aug 12, 2022

The above implementation is correct, and will only update the aria-live region once per call of the function.

Event propagation happens as a synchronous step, and the text part of the event is just a reference, it will not update the aria-live region upon setting. We use event propagation to allow for consumers to set a new text value, and then use that to update the aria-live region.

So currently the code does:

  • Something has changed so call updateFeedbackForScreenReaders(input)
  • Set the aria live container's text content to the input

Or to put it in pseudo code:

updateFeedbackForScreenReaders(input) {
  setAriaLiveContainerText(input)
}

The new behaviour I'm proposing is

  • Something has changed so call updateFeedbackForScreenReaders(input)
  • Create an event containing the input.
  • Emit the event, letting everyone know that a screenreader announcement is about to happen.
  • If the events input has changed, that is the new input to use.
  • Set the aria live container's text content to the input

Or to put it in pseudo code:

updateFeedbackForScreenReaders(input) {
  let event = {text: input}
  emitEvent(event)
  if (event.text !== input) input = event.text
  setAriaLiveContainerText(input)
}

@inkblotty
Copy link
Contributor Author

Ah @keithamus - Thank you! This makes more sense. It's definitely worth trying out the way you've built it here.

@brunoprietog
Copy link
Contributor

Hello @keithamus, thanks for the suggestion. However, the event to emit would have the text that updateFeedbackForScreenReaders method received. Wouldn't it be better for the event to emit a key? This key would be used to search I18n for the translated text according to the key sent. Relying on the text is very susceptible to change and is not always the same, for example when announcing the number of results found.

What do you think about defining the feedback texts in data attributes in the html? This would make it easier to send the text of the various feedbacks directly through the server.

@keithamus
Copy link
Member

keithamus commented Sep 28, 2022

It can emit more than just the text, but the text acts as a default and a pointer to replace it with something else. It would be straightforward for us to introduce more complex data in the event, for example here's one that takes a key, the default string and arbitrary per-event data:

class FeedbackEvent extends AutocompleteEvent {
  constructor(public key: string, public text: string, public data: Record<string, unknown>) {
    super('auto-complete-feedback', { bubbles: true, cancelable: true })
  }
}

// Somewhere in the code
const resultCount = 3
this.dispatchEvent(new FeedbackEvent('NEW_RESULTS', `${resultCount} new results`, { resultCount }))

This can then be replaced with userland code like so:

addEventListener('auto-complete-feedback', event => {
  if (event.key == 'NEW_RESULTS') {
    if (event.data.resultCount === 0) {
      event.text = my_i18n({ key: 'MY_KEY_NO_NEW_RESULTS' })
    } else {
      event.text = my_i18n({ key: 'MY_KEY_SOME_NEW_RESULTS' }, event.data)
    }
  }
})

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

No branches or pull requests

3 participants