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

WIP: add localizable dictionary #358

Closed
wants to merge 6 commits into from
Closed

WIP: add localizable dictionary #358

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d743b7f
WIP: add localizable dictionary
marcoscaceres May 2, 2017
bf5bcb0
Split up Localizable and TextDirection. (#1)
tobie May 2, 2017
381de47
fix some wording as suggested by Annevk
marcoscaceres May 3, 2017
97f6d52
Make it clear that it's one or more paragraphs
marcoscaceres May 3, 2017
bc58e24
members should be plural
marcoscaceres May 3, 2017
e78d5f5
Adds LocalizableString enum
marcoscaceres May 4, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions index.bs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12781,8 +12781,8 @@ Dictionaries that specify a [=localizable member=] must inherit from the {{Local
Specification authors must specify in prose which [=dictionary members|member(s)=]
of the inheriting dictionary serve as [=localizable members=].

It is recommended that specification authors limit the number of [=dictionary members=]
that are [=localizable member=], ideally to one.
Specification authors should limit the number of [=localizable members=] to one per dictionary,
including in [=inherited dictionaries=] (if any).
Copy link
Member Author

Choose a reason for hiding this comment

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

Is this bit about inherited dictionaries ok?

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 also fails to account for multiple paragraphs of a single member.

Said it's one or more. Does empty string count as one or zero?

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure. You'd have to check the bidi algorithm I'm afraid.

Copy link
Member Author

Choose a reason for hiding this comment

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

np, checking

Copy link
Member Author

Choose a reason for hiding this comment

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

If I'm reading correctly, a paragraph is denoted by the presence of a paragraph separator. However, it says that we can also define what a paragraph means: "Paragraphs may also be determined by higher-level protocols: for example, the text in two different cells of a table will be in different paragraphs".

Like we could say that a member's value constitutes a paragraph? Dunno. Opinions?

Copy link
Member Author

Choose a reason for hiding this comment

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

? That seems better than each consumer having to define its own dictionary.

Yeah, I quite the typedef and the flexibility it gives. Do you want to add it?

As such I think web payments should use my LocalizableString proposal instead of its dictionaries inheriting from Localizable. (In fact we can remove Localizable entirely and just have LocalizedValue.) We can then say Notifications' design is old-school.

Would be great!

Copy link
Member Author

Choose a reason for hiding this comment

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

@domenic, we can probably merge LocalizedValue into Localizable, or is there some reason to have it inherit from Localizable that I'm not seeing?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is what Payment Request would look like if using LocalizableString (😍):
https://github.com/w3c/browser-payment-api/pull/455/files

Also, LocalizableString means we can get rid of the "should" restriction, because now all localizable members now carry around their own lang, dir, and value 🎉.

Copy link
Member

Choose a reason for hiding this comment

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

Do you want to add it?

As in, do I think it's a good idea? Yes. Do I want to do the work myself? Well, I will if I have to, but not this week; I was hoping you could :)

we can probably merge LocalizedValue into Localizable, or is there some reason to have it inherit from Localizable that I'm not seeing?

Nope, let's merge them.

Copy link
Member Author

Choose a reason for hiding this comment

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

As in, do I think it's a good idea? Yes. Do I want to do the work myself? Well, I will if I have to, but not this week; I was hoping you could :)

Happy to!


<pre class="idl">
dictionary Localizable {
Expand All @@ -12798,7 +12798,7 @@ The empty string indicates that the primary language is unknown.
Any other string must be interpreted as a language tag.
Validity or well-formedness are not enforced. [[!BCP47]]

The {{Localizable/dir}} member provides the higher-level override of rules P2 and P3
For the [=localizable member=], the {{Localizable/dir}} member provides the higher-level override of rules P2 and P3
Copy link
Member

Choose a reason for hiding this comment

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

This needs to be plural. It's not just one.

if it has a value other than <a for=TextDirection enum-value>"auto"</a>. [[!BIDI]]
Copy link
Member

Choose a reason for hiding this comment

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

This should be more clearly made to apply to the localizable members.


<div class=example>
Expand Down