WICG / manifest-incubations

Before install prompt API for installing web applications
https://wicg.github.io/manifest-incubations/
Other
99 stars 29 forks source link

Adding User Preferences #57

Open aarongustafson opened 2 years ago

aarongustafson commented 2 years ago

Preview | Diff

marcoscaceres commented 2 years ago

Sorry, I think this is too complicated and quickly becomes unmanageable 😔 The translations and color scheme example hits at the problem.

I'd like to explore something like:

{
  "user_preferences": {
    "color-scheme: dark": {},
    "color-scheme: dark, language: es": {},
    "color-scheme: dark, language: fr": {}
  }
}

This could solve for the specificity problem. It flattens the structure into something manageable, and is quite easy to read and parse.

(If we can solve the above using CSS syntax, even better... then we don't need to create a new micro-syntax... however, it might not be unavoidable because we need to translate these values into things OSs can use, which is are not a CSS environment.)

loubrett commented 2 years ago

Combining this with translations is a good idea. I think we should avoid putting so much information into the keys though.

What about something like this:

"user_preferences": [
  {
    "color_scheme": "dark",
    "language": "es",
    "icons": [],
    "theme_color": ""
  }
]

This is very similar to one of your previous proposals in the original thread.

Or similar to @aarongustafson's proposal further down in the thread:

"user_preferences": [
  {
    "context": {
      "color_scheme": "dark",
      "language": "es"
    },
    "overrides": {}
  }
]
aarongustafson commented 2 years ago

I’m not averse to the idea of combining translations into the same block (though we may want to rethink "user_preferences" as a property in that instance), but I feel like there are a coup[le of things at play from a developer ergonomics, teachability & manageability standpoint that we should be considering as well, based on how these are most likely to be used. We should establish a set of guiding principles for ourselves in evaluating potential approaches.

I have some suggestions that I’m open to discussing/refining:

  1. Developers should be able to author overrides in as succinct a manner as possible.
  2. Developers should not be required to redefine properties they are not changing.
  3. The construct should make it clear what the context is and what is being overridden in that context.
  4. It must be clear to a developer what properties they are allowed to override in any context.
  5. Others??

Given all of this, I think I lean towards the approach @loubrett shared:

"user_preferences": [
  {
    "context": {
      "color_scheme": "dark",
      "language": "es"
    },
    "overrides": {}
  }
]

To map this to what @marcoscaceres was looking at earlier, to avoid the specificity problem, you’d end up with something like

"user_preferences_or_other_name_tbd": [
  {
    "context": {
      "color_scheme": "dark"
    },
    "overrides": {  }
  },
  {
    "context": {
      "color_scheme": "dark",
      "lang": "es"
    },
    "overrides": {  }
  },
  {
    "context": {
      "color_scheme": "dark",
      "lang": "fr"
    },
    "overrides": {  }
  }
]

This avoids the microsyntax problem and allows for expansion of the context block.

We would not need to deal with specificity as we can rely on known JSON/JavaScript behavior where redefining a property updates that value of that property if the context applies. In other words any subsequent override that applies in the context would trump earlier ones. By way of example, if each of the override blocks in the above code sample changed the background_color and the user was browsing in French and dark mode, the value from that block would apply. If the order was re-arranged, however, to up the dark mode only version last, that one would win.

The last thing we’d need to work out is whether redefining complex objects (e.g., shortcuts) replaces the whole value or whether we could allow for the overriding of specific parts of the object. I lean toward the latter. Incidentally, this is supported if you were to merge the original manifest object with the context-applicable override object using the spread operator:

updated_manifest = { …original_manifest, …overrides };
marcoscaceres commented 2 years ago

This avoids the microsyntax problem and allows for expansion of the context block.

Right, but it comes with the verbosity of having to add and ever growing list of new members: context, color_scheme, lang (which now becomes ambiguous, because we already have lang at the top level), etc. Effectively, that's just recreating what CSS MQ already convey.

If we just say, user_preferences and the members are just (media feature) media queries, then we don't need to define any new members and all the logic is deferred to MQs, along with the processing. As long as we say how the overrides work, then that solves for the problem.

Again, consider:

{
    "name": "game",
    "icons": [{}, {}],
    "user_preferences": {
        "prefers-color-scheme: dark and prefers-language: es": {
            "icons": [{}, {}],
            "name": "juego"
        },
        "prefers-color-scheme: dark and prefers-language: ja": {
            "icons": [{}, {}],
            "name": "ゲーム"
        }
    }
}

So, given the criteria:

  1. Developers should be able to author overrides in as succinct a manner as possible.

This holds. The MQ and the overrides don't require any nesting or new members. They only new thing they need to know is user_preferences and how to write the media query.

Again, compare the MQ solution to the context + overrides solution: the MQ solution is significantly more succinct.

Further, is solves for overrides where there is a context member missing, because having MQ as the key forces a context.

  1. Developers should not be required to redefine properties they are not changing.

This holds. Only overrides whatever is matched.

  1. The construct should make it clear what the context is and what is being overridden in that context.

This holds. The MQ makes the context clear. And the members listed are the override.

  1. It must be clear to a developer what properties they are allowed to override in any context.

This holds (or goes away), as there is no restriction. So it's always obvious.

aarongustafson commented 2 years ago

This holds (or goes away), as there is no restriction. So it's always obvious.

So are you thinking that every property would be open to updating, including id, start_url, etc.?

Are you open to atomic updates? For example, swapping only the name on a shortcut or a label on a screenshot?

marcoscaceres commented 2 years ago

So are you thinking that every property would be open to updating, including id, start_url, etc.?

As a starting point, yes. We do have a notion of "security sensitive members", but we would need to consider them on a case-by-case basis. For example, it might make sense to change the start_url to account for language where content negotiation is not possible (e.g., "start_url": "app/ja/" ).

What comes out after applying the user preferences is always a flat manifest, so it might be fine to just replace all things (including id, even if a bit of a foot-gun). The UA should deal with that and it also makes the behavior quite predictable.

Are you open to atomic updates? For example, swapping only the name on a shortcut or a label on a screenshot?

No. I think it adds too much complexity. I know the tradeoff is more redundancy, but I'd prefer we apply the "keep it simple" principle.

loubrett commented 2 years ago

I'd like to avoid parsing the keys. I'd be happy with the condition as a string but it should be a value since it needs parsing.

Maybe something like:

"user_preferences": [
  {
    "context": "prefers-color-scheme: dark and prefers-language: es",
    "overrides": {   }
  }
]

But I'm not sold on using media query syntax. I think it would be simpler to have a set of values we support for user preferences (eg color_scheme_dark, color_scheme_light, etc.) which can be used in combination with language codes for translations.

The last thing we’d need to work out is whether redefining complex objects (e.g., shortcuts) replaces the whole value or whether we could allow for the overriding of specific parts of the object.

I also lean towards allowing just overriding specifc parts of the object here but I can understand wanting to avoid that.

marcoscaceres commented 2 years ago

@loubrett, wrote:

I'd like to avoid parsing the keys.

Let's game this out a bit as we have shared goals. I might be missing something obvious so I'm going to ask a bunch of silly questions. Please bear with me. 🙏

Maybe something like:

"user_preferences": [
  {
    "context": "prefers-color-scheme: dark and prefers-language: es",
    "overrides": {   }
  }
]

Questions:

i.e., it's the same as:

"user_preferences": {
  // context
  "(prefers-color-scheme: dark) and (prefers-language: es)": {
    // overrides
  }
}

But I'm not sold on using media query syntax. I think it would be simpler to have a set of values we support for user preferences (eg color_scheme_dark, color_scheme_light, etc.) which can be used in combination with language codes for translations.

I kinda started at the same place and have bounced between MQ syntax and us specifying something new. The problem is that, when it's all put together, we basically end up at the same place: some kind of matching/query language that uses keywords/things that are already in CSS MQ. Consider the problem with "prefers dark mode" AND "prefers Spanish". To simultaneously express that with just JSON structures gets a little crazy with all the nesting. And coming up with our own thing is just reinventing the wheel, no?

So, my question is, if we have MQs already (and we have parsers in the browsers), and they both end up functionally at the same place, why not just use MQs?

marcoscaceres commented 2 years ago

Playing around a bit more... a "user_preferences" member probably doesn't make sense in the context of media queries. So, just an overrides member would work:

"overrides": {
  "(prefers-color-scheme: dark) and (prefers-language: es)": {
    "member": "some value"
  }
}
loubrett commented 2 years ago

I do like the simplicity of your syntax, but I feel that dictionary keys are not the right place to represent this data. I expect keys to be simple strings, not something that needs to be parsed.

do we expect to add more keys to the {context, overrides} tuple?

I'm not expecting more keys here, I was just trying to think of a way to turn the context string from a key into a value.

So, my question is, if we have MQs already (and we have parsers in the browsers), and they both end up functionally at the same place, why not just use MQs?

It will be more difficult to implement if we use MQs. @mgiuca brought up some issues on the original thread.

Playing around a bit more... a "user_preferences" member probably doesn't make sense in the context of media queries. So, just an overrides member would work:

I agree overrides sounds like a good name here if we go with that syntax.

aarongustafson commented 2 years ago

I like where things are heading in terms of simplifying the structure & approach. I am not against using the same structure as MQs and I think it would be up to the UA to decide how to handle things (i.e., whether they actually involve the MQ parser in determining applicability). Regardless we do need to be explicit about the fact that UAs will need to throw away overrides whose context includes unrecognized/unsupported strings/values.

I think it adds too much complexity. I know the tradeoff is more redundancy, but I'd prefer we apply the "keep it simple" principle.

Generally, I agree. I think where this really approach breaks down is redefining icons for shortcuts (and similar compound structures like file_handlers and — eventually, I hope — widgets). If all you need to do is swap the icon used when the app is rendered in dark mode, having to redefine the whole shortcuts object to do that seems incredibly painful and almost necessitates the use of a manifest builder script — which I know we’ve had a general principle of trying to avoid — in order to ensure it doesn’t get out of sync.

The mechanism for merging objects like this should be (I would think) easy to implement. To show how this could work, I put together this demo: https://codepen.io/aarongustafson/pen/eYMmrro. I’m sure the merge code could be optimized, it’s just a quick & dirty example.

More and more features that are on the horizon are complex JSON objects. Having to redefine each one as a whole is going to get quite unwieldly. Consider the following relatively simple example:

Following the "simple" approach, I would need to have the following override contexts defined over and above the original icons, definition, and shortcuts values and the arrays would need to be fully-redefined each and every time. And while it’s true the increase file size will come out in the wash when the file is compressed, it’s still incredibly cumbersome to maintain:

Compare this to supporting the more surgical approach:

"overrides": {
    "(prefers-color-scheme: dark)": {
      "background_color": "#5b5b5b",
      "icons": [
        { "src": "/i/icon-dark.svg" },
        { "src": "/i/icon-reverse.png" },
        { "src": "/i/notification-icon-reverse.png" }
      ]
    },
    "(prefers-language: no)": {
      "description": "Aaron Gustafsons hjem og arbeid på nettet.",
      "shortcuts": [
        { "name": "Notisbok" },
        { "name": "Snakker" },
        { "name": "Publikasjoner" },
        { "name": "Intervjuer" }
      ]
    }
  }

Now I’m not against allowing folks to define the whole complex object if they want to, but the more surgical approach is far cleaner and truly simpler for developers.