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.

Sign up for GitHub

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

Jump to bottom

Allow viewers to override system language #27023

Closed
newmuis opened this issue Feb 28, 2020 · 8 comments · Fixed by #28182
Closed

Allow viewers to override system language #27023

newmuis opened this issue Feb 28, 2020 · 8 comments · Fixed by #28182
Assignees
@Enriqe
Labels
P1: High Priority Type: Feature Request WG: stories
Projects
Stories - By Type Stories - By Effort

Comments

@newmuis
Copy link
Contributor

newmuis commented Feb 28, 2020

Stories currently use the document language to internationalize the system layer. We should allow viewers to override the document language, if desired.

This can just be a fragment parameter e.g. #lang=es
@newmuis newmuis self-assigned this Feb 28, 2020
@newmuis newmuis added this to Incoming (Untriaged) in Stories - By Type via automation Feb 28, 2020
@newmuis newmuis added this to Untriaged in Stories - By Effort via automation Feb 28, 2020
@newmuis
Copy link
Contributor Author

newmuis commented Feb 28, 2020

A simple design could be something like propagating a fragment parameter lang to an attribute viewer-lang on the root <html> node of the document. Scripts can then read from that node, if they like.

/cc @ampproject/wg-ui-and-a11y is this generally useful elsewhere? If there's no interest otherwise, we can go ahead with an implementation for stories. Otherwise, we can take additional requirements into consideration, if there are any.

@nainar
Copy link
Contributor

nainar commented Mar 2, 2020

I haven't seen any specific requests for this here on GitHub or via individual developers using AMP. However, this sounds globally useful outside of AMP. Do we know if any discussions have taken place around this? Or via more people would do this in stories as opposed to other web content?

@newmuis
Copy link
Contributor Author

newmuis commented Mar 2, 2020

We have specific requests for this in the stories context, due to the ecosystem-wide swipe-to-next-story paradigm (and the desire to keep the system language the same across stories)

@alanorozco
Copy link
Member

alanorozco commented Mar 2, 2020 •
edited

I think the model to get this working on standard AMP would be very different from stories.

amp-story render blocks, and renders all these strings on runtime, so the fragment annotation/viewer messaging API would work fine.

Outside amp-story, components that are users of this would have to either render-block or "refresh" their UI with language strings, none of which would be a great experience.

Like @nainar mentioned, the overall i18n problem is not particularly severe for other components (maybe in the future?) since they rarely render labeled chrome like amp-story does.

Since the models for stories and other document types don't match, my request is to do one of these options:

  • A. Redesign this API to replace language tokens to be replaced as part of a document transform, and allow the viewer to use a separate document for a specific language. This is a lot more involved and I don't see a lot of need for it, to be honest, or...:

  • B. Namespace this either via a prefix, like #amp-story-lang=en, or an amp-story specific URL property for state, like #amp-story={"lang":"en"}. This siloes the API so we don't expect it to work outside of stories, because it won't. (I like the single object prop better, since it'd also be easier to propagate configuration when nested, like with <amp-story-player>)

@newmuis
Copy link
Contributor Author

newmuis commented Mar 2, 2020

Can we do Option C: a forked implementation using a non-namespaced parameter? i.e. we go ahead now, using lang and expecting a language code, and if/when other AMP flavors ever need it, the API is there, but the implementation can be arbitrary on the runtime side?

@newmuis newmuis mentioned this issue Mar 2, 2020
Closed
@alanorozco
Copy link
Member

@newmuis The problem is that a fragment can't work for non-render blocking cases :)

By forked implementation, what level are we talking about? amp-viewer-integration?

@newmuis
Copy link
Contributor Author

newmuis commented Mar 2, 2020

It can work for anything that is rendered after the initial page load, or anything that we transform on the cache. But, mostly I want to YAGNI and make sure we don't block on considerations for features that don't exist, especially if it's possible to migrate down the line 🙂

@gmajoulet gmajoulet assigned Enriqe and unassigned newmuis Apr 22, 2020
@gmajoulet
Copy link
Contributor

Reassigned to @Enriqe

@Enriqe Enriqe mentioned this issue Apr 23, 2020
Merged
@Enriqe Enriqe mentioned this issue May 4, 2020
Merged
Stories - By Type automation moved this from Incoming (Untriaged) to Done May 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Enriqe Enriqe

Labels
P1: High Priority Type: Feature Request WG: stories
Projects
Stories - By Effort
Untriaged
Stories - By Type
  
Done
Milestone
No milestone
5 participants
@alanorozco @gmajoulet @newmuis @Enriqe @nainar