-
Notifications
You must be signed in to change notification settings - Fork 334
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
Research a data-attribute approach to passing translations to component JavaScript #2736
Comments
Immediate issues that come to mind that warrant investigation:
It might also be worth investigating the feasability of using encoded JSON, to allow for non-string data structures within the data-* attribute. This might let us cut down on the extreme cases of components having dozens of i18n data-* attributes. |
Bootstrap are experimenting with this: https://getbootstrap.com/docs/5.2/getting-started/javascript/#data-attributes It looks like all their data attributes can now house JSON values. |
Technical limitationsElement attributes length and count:HTML5An attribute value is a string. Except where otherwise specified, attribute values on HTML elements may be any string value, including the empty string, and there is no restriction on what text can be specified in such attribute values Every HTML element may have any number of custom data attributes specified, with any value. https://jsbin.com/wikulamuni/3/edit?html,js,output This script works fine in Chrome with an attribute value length of 10 million+ characters. HTML4HTML4 allows 60 attributes with a total of 65536 characters. Attribute values restricted charactersAttribute values are a mixture of text and character references, except with the additional restriction that the text cannot contain an ambiguous ampersand. Other limitations
ImplementationI have a spike branch here: https://github.com/alphagov/govuk-frontend/compare/bk-spike-i18n-data-attributes I think this is non-breaking after the i18n stuff has been added, so wouldn't necessarily have to be targetted for the next major release change. getOptionsI've added a
It's in a very rough state (see comments in the file), but works for the minimal example... exampleI've added an example using the Accordion. I've pasted the HTML and added data attributes (Nunjucks macros would simply take params and use them as data attributes internally). Further workOn this spike
If implemented
|
Thanks @domoscargin, I think the first two in the order of precedence should be reversed. It doesn't seem like there are likely we'll hit any technical restrictions that affect the use of I like the idea of having an inherited base class that includes this and other programatic API work. It feels like a more scalable way to handle it going forwards. I'm still curious as to how this may handle features such as embedded HTML or nested JSON. Good work so far! Interested to see where it goes. |
I agree with this, but was wavering because the proposal includes programmatic initialisation of an individual component. I considered somehow making the priority individual programmatic component -> data attributes -> initAll -> default, but splitting out the programmatic options like that seems like a recipe for confusion. |
Thanks for the really clear write-up @domoscargin , great work 👍 The general gist I'm getting is that there's no real technical limitations that would stop us from doing this, so it may come down to other things, primarily if it's what users need/find useful, how we'd document it (especially if it sits alongside the existing approach for passing translations). I agree with @querkmachine on the specificity order - I think my assumption was the reversed order, with data attributes taking priority (thinking of them a bit like inline styles or our override classes), but that's an implementation detail we can get into if we're thinking of going with this approach. I don't think it's worth getting too in depth until we know it's something we're going to do (and we'll need to wait for our user feedback before making any decisions, I think), but probably worth having a quick play around with the things Kim has mentioned to make sure none of them are going to cause us issues:
|
I thought I'd try out the JSON approach in a separate branch here ➡️ View spike branch here Seems technically feasible to implement, but I wonder if it fully resolves the issue of a user needing to understand where a string is used. It still partly separates out the JavaScript strings from the Nunjucks strings, so while it feels easier because it's all in one place, you'd still probably need to consult the macro documentation to understand what goes where. It also might feel weird to anyone using the HTML versions, because adding JSON into a data attribute isn't something you do very often. Having said that, I do quite like it as an alternative to our existing approach, because it feels nicer to have it all in one place. @domoscargin give me a shout if you'd like to talk about this spike when you're back! Happy to chat and figure out where we go from here |
Thanks for this, @vanitabarrett! So it seems like, in terms of the questions we're asking on this spike:
I think these comments are pretty easy to follow, so we could ask folks to glance at it and then call this done, or I could write up a summary? |
@domoscargin I think that's probably a good summary in itself! Worth noting that "Separates Nunjucks and JavaScript strings so user has to figure out where things go" only really applies if going with the approach where a user passes in a JSON object for translations. It wouldn't be the case if we separated out each individual translation string into it's own macro param, although that has it's own downside where you may have a lot of data-attributes on the component. |
Had a little play with an alternative approach to passing the JSON, by sticking it in an inline (Spikes on spikes on spikes at this point?) |
We have decided we're going to go with a data-attribute approach for passing translations, alongside the currently proposed JS approach. I'm going to close this spike and write up some new cards for doing the work. |
What
Research into how a data-attribute based approach to passing translations for strings in our component JavaScript would work. This might be in isolation and/or alongside another approach, like passing a translations object through initAll() at the point of initialising.
We want to know if this approach feels feasible (even if it may be untidy, in terms of a lot of data attributes), or if it's a non-starter.
Why
We're unsure if our current planned approach for passing translations to our component JavaScript will work well for all our users.
We want to know if there are any technical limitations or strong downsides to a data-attributes approach that would mean it's not appropriate for our use case (even if working alongside our current planned approach)
Assumptions
Timebox
1 day
We should review progress after this period of time has elapsed, even if the spike has not been 'completed'
Who is working on this?
Spike lead: @domoscargin
Spike buddy: @vanitabarrett
Questions to answer
Done when
You may find it helpful to refer to our expected outcomes of spikes.
The text was updated successfully, but these errors were encountered: