google / physical-web

The Physical Web: walk up and use anything
http://physical-web.org
Apache License 2.0
6k stars 666 forks source link

Show icons from Web App Manifest #620

Open kenchris opened 8 years ago

kenchris commented 8 years ago

The Physical Web list in Chrome doesn't make use of the icons in the manifest files used by Progressive Web Apps.

scottjenson commented 8 years ago

So just to be technically clear, is the only thing we have to do is look for an app manifest? I'd like to get a clear technical answer as to how this info is presented as there is a tiny worry there may be more than one way to do it.

cco3 commented 8 years ago

Right, the webapp manifest would override the standard favicon. https://developers.google.com/web/updates/2014/11/Support-for-installable-web-apps-with-webapp-manifest-in-chrome-38-for-Android

Implementation-wise, this would only be a change at the PWS level. I'll assign Michal, and he can reassign if he wants.

scottjenson commented 8 years ago

The current spec does not specify a Description attribute but there is an issue filed to add it Worst case we only set icon/title with the manifest but maybe we can hopefully look for it? (or is that bad form?)

kenchris commented 8 years ago

In the future, it might make sense to launch the URLs as progressive web apps (honoring "standalone", orientation lock, theme_color etc) in the case the announced url and start_url match (hash and query might not necessarily need to match), but that would need to be well thought out. @owencm @PaulKinlan

scottjenson commented 8 years ago

Agree, the Physical Web is about finding the PWApps but once found, we hope they are saved and then used on their own. This issue is simply saying that if there is a WebManifest (whether or not it is a full fledged PWApp), let's honor that information.

mmocny commented 8 years ago

Hello Again @kenchris!

I think that web app manifest name/short_name are poor substitutes for page <title> for most deep links, and as Scott mentions, description is missing entirely. We should certainly use it as a fallback for favicon.

I really like the suggestion to leverage manifest more strongly if the beacon url == start_url (minus url fragment, possibly query param).

FYI, the open source PWS on this github already supports web app manifest. We actually currently use it over the page tags, and I think that should change (for reasons mentioned).

Also, for chrome bugs specifically we have a crbug.com component called Internals>PhysicalWeb. I'm OK with filing issues here for discussion and letting us file the bugs there, but for simple Chrome-specific issues that's the right place to file.

kenchris commented 8 years ago

Hi @mmocny,

(ccing @PaulKinlan)

I somehow missed your comment :-) Sorry about that.

If the beacon url is equal to the start_url I definitely think we should use the manifest. We are currently looking at adding "description" to the manifest, but we need to know whether you will use it in that case.

Given my own experience with PWAs and Physical Web, I think that would be a positive change.

Given that the manifest has a scope variable, it probably might make sense to leverage the manifest for any URL inside the scope, but I think that more discussion for this is needed.

I didn't file this in crbug as others (like Opera etc) might be interested in this discussion.

scottjenson commented 8 years ago

The key issue I've heard about using description is how applicable it is for sub pages. I think it boils down to how much we trust the PWS and it's manifest. I'd like for there to be clear rules here. Here is a strawman:

If there is a manifest AND it has a description, we honor that as the dev put it there. This will work for all URLs within the PWA If the dev wants per page descriptions, don't fill in the manifest and we'll use tag info for each page

Does that work? Any issues?

kenchris commented 8 years ago

I think that would work, so to clarify.

This becomes the source of truth for the url in start_url (if defined) and urls within scope if defined, and thus not for the url linking to the manifest unless covered by these,

When we decide on this, I would like Opera (@brucelawson) to follow and we should write an article and put it on developer.google.com (@paulkinlan).

mmocny commented 8 years ago

For all pages that are within the scope, I really don't think we should use the manifest values (at least by default). It would mean a bland generic app descriptor instead of a specific page snippet.

If developers want all pages within the scope to use the same metadata, they can just do so by setting <title> etc tags accordingly. In the spirit of PWA's being "progressive", I think developers shouldn't be relying on one manifest for metadata descriptors for all pages.

If we do think its important to allow the manifest to override for all pages within the scope, here are two strawman proposals:

{
    ...
    "physicalweb_settings": {
        "use_manifest_metadata_for_all_pages_within_scope": true
    }
    ...
}

The former seems easier for developers, the latter is more flexible (and possibly more efficient).

Honestly, I don't see the need to rush into this and document it for the long term. In-page metadata tags have worked well for years.

kenchris commented 8 years ago

For most single-page apps, relying on the page snippets can result in pretty bad descriptions. That is at least my experience from my own apps, so I do think that using the manifest for those kind of apps makes sense.

In single-page apps, most urls will actually not be actual pages but kind of generated content.

kenchris commented 8 years ago

So, what about using title and meta description if they exists, or-else if manifest exists and url is within scope, fall back to info in manifest?

mmocny commented 8 years ago

We don't currently deal well with locally generated content, anyway. So in this case we would probably use the landing page metadata for all deep links to single page apps, ignoring the url fragment. And in this case, I expect that means url == start_url, and we would use the manifest with priority.

Totally agree that for URLs "within scope", using manifest as a fallback makes total sense. However, in developer tools (e.g. apps that program beacon values), we may wish to flag this as a warning so deployers know what's going on.

kenchris commented 8 years ago

Sounds good. @marcoscaceres it seems that we can move ahead and add description.

marcoscaceres commented 8 years ago

Ok, will draft it up.

marcoscaceres commented 8 years ago

@scottjenson, @kenchris, please review https://github.com/w3c/manifest/pull/456

marcoscaceres commented 8 years ago

And @mmocny 😄