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

Derive direction from document element too #956

Merged
merged 5 commits into from
May 20, 2021
Merged

Derive direction from document element too #956

merged 5 commits into from
May 20, 2021

Conversation

marcoscaceres
Copy link
Member

@marcoscaceres marcoscaceres commented May 5, 2021

Related to, but does not close #327, #946, #948, #951

The following tasks have been completed:

  • Confirmed there are no ReSpec errors/warnings.
  • Modified Web platform tests (link)
  • Modified MDN Docs (link)
  • Has undergone security/privacy review (link)

Implementation commitment:

  • Safari (link to issue)
  • Chrome (link to issue)
  • Firefox (link to issue)
  • Edge (public signal)

Optional, impact on Payment Handler spec?


Preview | Diff

@marcoscaceres marcoscaceres requested a review from ianbjacobs May 5, 2021 06:32
@ianbjacobs
Copy link
Collaborator

ianbjacobs commented May 5, 2021

Hi @marcoscaceres,

Thanks for starting this. Some thoughts:

  1. The phrase "payment sheet" is not used elsewhere in the document. I think we should either define it (e.g., by adding "(called the 'payment sheet')" after "Present a user interface" at the beginning of item 18) or remove it. If we remove it, then the note title could become: "Localization of the user interface to interact with handlers". Then you could just s/payment sheet/user interface/ in the note body.

  2. Point 18 says "SHOULD use info provided in content." It's not clear to me that the note needs to say "As such, user agents will generally present the payment sheet using the user agent's default language." That to me suggests that despite the recommendation we just made, nobody is doing it.

What about a Note that says:

"This API relies on inherited localization information. The working group is considering support for fine-grained localization of the user interface and of individual PaymentItems in a future version of this API."

Copy link
Collaborator

@ianbjacobs ianbjacobs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(See my comments above)

@marcoscaceres
Copy link
Member Author

That to me suggests that despite the recommendation we just made, nobody is doing it.

That's the truth tho :)

Will update based on your suggestions.

@ianbjacobs
Copy link
Collaborator

@aphillips, would you mind having a look at this proposal to address the lack of fine-grain localization in V1 of the API? Thank you!

Copy link

@aphillips aphillips left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally looks good. Small tweaks proposed.

index.html Outdated
SHOULD be presented using the language, base direction, and
locale-based formatting that matches the |document|'s [=document
element|document element's=] [=Node/language=] and
{{Document/dir}} attribute, if any, or an appropriate fallback

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The direction should also be at the Node level, no?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should, but there is no such thing on Node unfortunately:
https://dom.spec.whatwg.org/#node

The only "dir"s in the platform are:
https://respec.org/xref/?term=dir

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, wait... we might be able to use https://html.spec.whatwg.org/multipage/dom.html#the-directionality ... but I'll need to get it "exported" so we can use it in our spec.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sent whatwg/html#6676 ...

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@marcoscaceres Thanks. I squinted at {{HTMLElement/dir}} and it wasn't clear to me if it included inherited direction. [^html-global/dir^] is the definition of the dir attribute and includes inheritance, but doesn't seem quite to match our needs here. Exporting directionality is a better solution--not just here but generally! I may take whatwg/html#6676 to expedite ;-).

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LOL. I thought 6676 was an issue, but it's a PR...

Copy link
Member Author

@marcoscaceres marcoscaceres May 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aphillips, take a look at @annevk's comments at whatwg/html#6676 ... I think they make a lot of sense.

My feeling is that we punt on the direction for now, and solve this with "localizable" in a future version of the spec.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aphillips, would that work for you?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@marcoscaceres I'm pondering a reply to @annevk: I think he is right, although I18N does want to promote using the document's context when it is available and is appropriate.

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
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.

Add dir/lang member on dictionaries whose content is displayed in browser UI
3 participants