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

languagePicker: 'change language' popup needs a lang attribute #2785

Closed
moloko opened this issue May 28, 2020 · 11 comments · Fixed by #2786
Closed

languagePicker: 'change language' popup needs a lang attribute #2785

moloko opened this issue May 28, 2020 · 11 comments · Fixed by #2786

Comments

@moloko
Copy link
Contributor

moloko commented May 28, 2020

Because the text of the popup is in the language the learner is trying to switch to, it is therefore different to the language declared via the lang attribute of the html element.

It should be possible to add lang attribute to Notify that specifies the language used in the popup correctly. This would be beneficial both for assistive technology and for language-specific styling as well (for example, you may want to use a different font for non-Latin languages)

@guywillis
Copy link
Contributor

Ideally alongside the lang attribute there should be a language class is-en / is-de to amend styling / font is required.

@moloko
Copy link
Contributor Author

moloko commented May 28, 2020

Ideally alongside the lang attribute there should be a language class is-en / is-de to amend styling / font is required.

small suggestion: something more like is-lang-de might be a bit clearer? otherwise someone might look at a class like is-de and wonder what on earth it means! Not to mention is-it for Italian...

@NayanKhedkar
Copy link
Member

I think class will be a fine. Is there attribute really need? PR

@tomgreenfield
Copy link
Contributor

As @moloko says, the attribute is useful for assistive technology.

@moloko
Copy link
Contributor Author

moloko commented Jun 1, 2020

@NayanKhedkar Jaws uses the lang attribute to work out what language to use when reading out the text

@NayanKhedkar
Copy link
Member

Hi, Should I write here

attributes: function () {
      return _.extend({
        'role': 'dialog',
        'aria-labelledby': 'notify-heading',
        'aria-modal': 'true'
      }, this.model.get('_attributes') || {})
 }

and here
_attributes: { 'lang': newLanguage }

@tomgreenfield
Copy link
Contributor

A few things to note about the above code:

  • Object.assign() can be used since we now have a polyfill for IE
  • No || {} fallback is needed because Object.assign() handles falsies
  • lang doesn't need quotes

@NayanKhedkar NayanKhedkar linked a pull request Jun 2, 2020 that will close this issue
@NayanKhedkar
Copy link
Member

Dependent PR:

@moloko
Copy link
Contributor Author

moloko commented Jun 25, 2020

not actually fixed yet..!

@guywillis
Copy link
Contributor

I think this issue can be closed now?

@moloko
Copy link
Contributor Author

moloko commented Jul 7, 2020

yep, done in FW v5.6 and languagePicker v4.1.0

@moloko moloko closed this as completed Jul 7, 2020
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 a pull request may close this issue.

4 participants