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

Allow custom accordion open and close text #1328

Closed
wants to merge 1 commit into from

Conversation

HughePaul
Copy link

Try to read custom open and close button text from a data attribute and fall back to English text to enable localisation.

fix for #1325

Try to read custom open and close button text from a data attribute and fall back to English text to enable localisation.
@NickColley NickColley added the awaiting triage Needs triaging by team label May 9, 2019
@kellylee-gds kellylee-gds removed the awaiting triage Needs triaging by team label May 15, 2019
@frankieroberto
Copy link
Contributor

Nice! Feels a bit weird to be including html fragments within attributes (as you'd have to remember to escape any " marks) but maybe that’s ok.

The alternative might be to bake multi-language support (at least for Welsh?) into the javascript, and then use a data-lang (or just lang?) attribute to select the right language.

@NickColley NickColley added the awaiting triage Needs triaging by team label Jun 13, 2019
@kellylee-gds kellylee-gds removed the awaiting triage Needs triaging by team label Jun 19, 2019
@hannalaakso
Copy link
Member

Hi @HughePaul

Thanks a lot for your work on this 🙌 This is a totally valid approach which solves your particular use case, but we need to find a way to support localisation consistently in a way that will work for different types of components and for other languages. For that reason, we’re not going to be able to accept this pull request.

There's some related discussion here for context: #958 (comment).

We've captured your PR as part of a larger epic #1389 to improve how Javascript works in GOV.UK Frontend. For now, we suggest that you fork and vendor in this component and make the adjustments in the service you’re working on.

Thanks for taking the time to propose this, and for your patience.

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.

5 participants