-
Notifications
You must be signed in to change notification settings - Fork 135
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
l10n: adds Localizable dict + TextDirection enum #455
Conversation
WIP, seeking feedback. It's just missing a statement about how to use the information when presenting the strings in a UI. |
index.html
Outdated
@@ -1108,7 +1108,7 @@ | |||
<dfn>PaymentDetailsBase</dfn> dictionary | |||
</h2> | |||
<pre class="idl"> | |||
dictionary PaymentDetailsBase { | |||
dictionary PaymentDetailsBase : Localizable { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want this to apply to PaymentDetailsUpdate.error
, which is why put it here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it should go here if it only applies to PaymentDetailsUpdate. Instead you're going to have to just duplicate the two things (or use a partial dictionary PaymentDetailsUpdate).
If you put it here then PaymentDetailsInit gets it, and in particular that means passing { get dir() { throw new Error("boo"); }
to places that expect a PaymentDetailsInit must throw.
index.html
Outdated
member that will be displayed to the user. For example, this might | ||
commonly be used to explain why goods cannot be shipped to the chosen | ||
shipping address. | ||
<a>PaymentDetailsUpdate</a> can contain a human-readable message in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm using "human-readable" as the key that binds together all these "localizable" members. Need to formally define "human-readable" as being potentially affected by dir/lang when present.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
("human-readable" was already in use in the spec, which is why I reused it - maybe we can come up with something nicer)
Would be good to land after #451 to avoid conflicts. Notably this does not allow serving up multiple options with different locales and allowing the browser to choose between them. I guess that is a non-goal. It's also noteworthy that this matches NotificationOptions already, so that's good. Maybe we can consolidate this Also, this standard says it represents a BCP 47 language tag, but that's not quite accurate; a developer could pass in "asdf" and there's no normative text rejecting that. You'll instead want something similar to the verbiage at https://notifications.spec.whatwg.org/#language. |
Yeah. Non-goal I think.
I was thinking the same.... would be great to have this in Web IDL. Maybe we can craft
Will fix.
Yeah, that's some good wording. |
Uplifting to IDL seems reasonable. We already put some shared concepts and processing models there. |
Yeah, seems this could go in §4 Common definitions. |
@domenic wrote: "Notably this does not allow serving up multiple options with different locales and allowing the browser to choose between them." If I understand your comment, it is about support for multiple strings in multiple languages. (If you were talking about something else; sorry for the noise.) Ian |
Yeah, it makes sense. So this is mostly about allowing people to markup certain messages as having a different lang/dir than the rest of the content on the page. |
index.html
Outdated
<dfn>auto</dfn> | ||
</dt> | ||
<dd> | ||
Directionality is determined by the [[!BIDI]] algorithm. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The bidi algorithm should be used for ltr
and rtl
as well... right?
I think we basically want to hook into http://www.unicode.org/reports/tr9/#Explicit_Directional_Isolates similarly to what CSS does in https://drafts.csswg.org/css-writing-modes-3/#unicode-bidi (for isolate
or plaintext
-- not sure which is more appropriate here).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://notifications.spec.whatwg.org/#direction has the text you need for this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, will try to draft it soon.
cc @w3c/i18n-reviewers |
index.html
Outdated
@@ -1108,7 +1108,7 @@ | |||
<dfn>PaymentDetailsBase</dfn> dictionary | |||
</h2> | |||
<pre class="idl"> | |||
dictionary PaymentDetailsBase { | |||
dictionary PaymentDetailsBase : Localizable { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it should go here if it only applies to PaymentDetailsUpdate. Instead you're going to have to just duplicate the two things (or use a partial dictionary PaymentDetailsUpdate).
If you put it here then PaymentDetailsInit gets it, and in particular that means passing { get dir() { throw new Error("boo"); }
to places that expect a PaymentDetailsInit must throw.
index.html
Outdated
</pre> | ||
<p> | ||
The text-direction values are the following, implying that the value of | ||
the human-readable members is by default: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sentence doesn't make sense to me.
blocked on whatwg/webidl#358 |
argh, that's not right. Seems I screwed up a rebase. |
better... |
5e4824f
to
3ee967a
Compare
Still blocked on discussions over in the WebIDL repo. Probably take another few days to resolve those. |
@@ -2810,6 +2810,10 @@ | |||
</dt> | |||
<dd> | |||
<p> | |||
The <code><dfn>LocalizableString</dfn></code> <a data-cite= | |||
"!WEBIDL-LS#dfn-typedef">typedef</a> is defined by [[!WEBIDL-LS]]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can go ahead and just use [[WEBIDL]], btw.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will fix globally! Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[[WEBIDL]] is informative reference, [[!WEBIDL]] is normative reference.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant WEBIDL vs WEBIDL-LS, sorry if that caused confusion. :(((
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's all good. I sent #524
Closing for now... will reopen if we want to add this in v2 |
PaymentShippingOption
andPaymentItem
inherits from WebIDLLocalizable
label
members of the above are defined as localizable membersPreview | Diff