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

Adding User Preferences #57

Draft
wants to merge 9 commits into
base: gh-pages
Choose a base branch
from
Draft

Adding User Preferences #57

Changes from 2 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
37cdf63
Initial pass: Manifest Override
aarongustafson Jun 13, 2022
6b4b389
Update index.html
aarongustafson Jun 13, 2022
facdebe
Add user_preferences member
loubrett Jun 14, 2022
e94d18d
Update index.html
aarongustafson Jun 14, 2022
0e12379
Update index.html
loubrett Jun 15, 2022
4cbfa14
Update index.html
loubrett Jun 15, 2022
87125c3
Add extra nesting to user_preferences
loubrett Jun 16, 2022
449b6d3
Update index.html
loubrett Jun 16, 2022
1cc9d12
Adding an algorithm for processing an override object
aarongustafson Jun 24, 2022
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
133 changes: 133 additions & 0 deletions index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,139 @@ <h2>
</p>
</section>
</section>
<section data-cite="image-resource">
<h2>
Manifest override objects
</h2>
<p>
Each <dfn>manifest override object</dfn> is a generic object value
that allows for certain manifest properties to be overridden within
a particular context.
</p>
<p>The structure of a [=manifest override object=] is as follows:</p>
<pre class="json">
{
"context_key": {
"property": "new value"
}
}
</pre>
<p>
Each manifest property that accepts a [=manifest override object=]
as its value will define the contexts it supports and which
properties it supports overriding. User agents MUST ignore any contexts
that are not supported by the property as well as any override
properties not explicitly allowed within it.
</p>
<p>
When the manifest property’s context is applicable, the value of each
allowable override will be used in place of the original value defined
in the Manifest.
</p>
<p>
Redefined array items will be overridden in the order they are authored.
When redefining objects (e.g., [=manifest/shortcuts=], [=manifest/icons=]),
authors will only be able to redefine specific properties of that object.
In order to ensure all overrides are applied correctly, the order must
match the original array (i.e., each [=manifest/shortcut=] must be redefined
in order, as must their icons, if they also require re-definition).
</p>
Redefined array items must also be equal in number to the array being
overridden. If there is a mismatch in the number of items in either array,
any excess items will be ignored. This is only an issue if the original
array has more items than the override array, because any excess items
within the original array will not be re-defined.
</p>
<p>
When there is a conflict because two different properties are attempting
to override the same value in their respective active contexts, the one
defined last will win. By way of example, consider the following:
</p>
<pre class="json">
{
"lang": "en-US",
"icons": [
{
"src": "icon.png",
"sizes": "128x128",
"type": "image/png"
}
],
"translations": {
"es": {
"icons": [
{
"src": "icon-es.png"
}
]
}
},
"user_preferences": {
"color_scheme_dark": {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is nested one level deeper now, see https://github.com/WICG/manifest-incubations/blob/gh-pages/user-preferences-explainer.md for the latest version.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I did see that and wanted to discuss: Would the manifest override construct hold up if there’s another level of nesting? I think the property > context = overrides makes sense, but in the revised user prefs model, it’s more like property > property > value = overrides. Feels inconsistent. In my view of these two features, having a single context to map to makes more sense.

Couldn’t the same thing be achieved by making the context "color_scheme_VALUE" (as we had it and how I used it) or "color_scheme: value" (or something similar)? What does the additional nesting give us in exchange for the cognitive overhead of having to manage more curly braces?

Copy link
Contributor

Choose a reason for hiding this comment

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

IIRC this kind of nesting was strongly recommended by the TAG.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, the extra nesting was wanted by TAG.

I wonder if we need to define this as property > context > overrides? Could we just define the overrides part? Maybe we will also want to use this in future fields that don't fit this nesting?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I would love a pointer to that TAG conversation (just for backstory). I thought on it a bit more and I think I have reasoned it out. user_preferences has its own set of properties and those properties take a manifest override object. So the structure ends up being more like property > sub-property > context = overrides. It still feels unnecessarily complex (on another thread @marcoscaceres was actually pushing for flatter Manifest structures), but I can make it work from a spec standpoint.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is TAG review here: w3ctag/design-reviews#696

I don't have a strong preference as to which structure we use, as long as the TAG reviewers are on board.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for that. I'm good with the adjusted structure and I think we just define them (e.g., color_scheme) as sub-properties whose values are a manifest override object. That does give us the extensibility for contrast, etc. too. Thanks for giving me the space to think through it a little more.

Can you update my conflict example accordingly or do you want me to do it?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for thinking that through, that sounds good to me.

Yep, I can update that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Awesome thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

As an ex-TAG member, I'm not sure the TAG's opinions has more weight than anyone else's. Their guidance is always valuable and welcome, but we are free to explore a range of solutions that are best for developers.

"icons": [
{
"src": "icon-dark.png"
}
]
}
}
}
</pre>
<p>
In this example, if the user’s primary language is Español, but
their preferred color scheme was set to "dark", the icon supplied
would be the dark version and not the localized one. For this reason,
it is imperative that properties taking a [=manifest override object=]
as their value consider whether any other properties that also enable
overrides should be able to be redefined within them. In the above
example, the author’s intent would likely have been better realized
if `user_preference` was put before `translations` and the localized
aarongustafson marked this conversation as resolved.
Show resolved Hide resolved
context block redefined the `user_preferences` value for that language:
</p>
<pre class="json">
{
"lang": "en-US",
"icons": [
{
"src": "icon.png",
"sizes": "128x128",
"type": "image/png"
}
],
"user_preferences": {
"color_scheme_dark": {
"icons": [
{
"src": "icon-dark.png"
}
]
}
},
"translations": {
"es": {
"icons": [
{
"src": "icon-es.png"
}
],
"user_preferences": {
"color_scheme_dark": {
"icons": [
{
"src": "icon-es-dark.png"
}
]
}
}
}
},

}
</pre>
<p>
User agents MAY ignore any override properties they do not support.
</p>
</section>
<section data-cite="DOM">
<h2>
Installation prompts
Expand Down