Add manifest.json file

I was wondering where the
and manifest file actually were in this module. Seems like a core requirement for PWA?

Patch doesn't apply for me. Here's a new version that I've prepared and am currently testing.

I found out recently that there is a contrib module called Responsive Favicons which appears to provide everything needed for serving a manifest file and home screen icons icons of various sizes.

It provides Drupal integration for files produced with an online service called Real Favicon Generator, which as well as icons, also provides a manifest file.

Agree, better to have it all in core, but as we know this can take a while. In the meantime I believe this module provides a very complete solution.

Would it make sense to check the minor version of Drupal core and provide the manifest until a time where it's clear that core is providing the file?

in D7 you just do something like

if (VERSION >= 7.23) {
  // do it the NEW way as of 7.23
} else {
  // do it the old way from 7.0-7.22
}

@ArnoVDC what can I do to help move this forward? Can you provide some instructions for me to test this? I'm fairly inexperienced with D8 and any help you can provide will be very appreciated.

Ok I started by just applying the patch. UI is looking good!

• I dig the color input, nice use of HTML5 inputs. 👍
• I would say that "Display type" needs to be changed to "Display mode" per MDN display docs. I'd personally default to minimal-ui instead of fullscreen/standalone because most Drupal sites are not apps, rather they are content-driven websites.
• The Content-Type for the manifest file needs to be application/manifest+json per core discussion: #2698127-76: Extend site information page to generate a manifest.json file
• Orientation would be a good option to add. There are a set of predefined options to use.

For image uploads I see the 512x512 option is hardcoded and that's the right direction. Would it make sense to add a second field for 192x192? At the time of writing those are the only two sizes required by most UAs to prompt add-to-homescreen, per Lighthouse/Google PWA guidelines for manifest icons.

We'll also need to have an administer pwa permission as part of this patch. Curiously, the permission is already included in the routing file, but doesn't exist in the permissions config.

Adding a related issue. The two seems to be dupes of each other. The other issue is older but one way or another maybe we can combine the two efforts?

Alright after a little more searching we figured out that minor releases of core can be specified for contrib compatibility. We can commit the manifest code, set the compat to <= 8.6, and then whenever the manifest is committed to core we can change the contrib module to say >= 8.X (whatever it ends up being.. hopefully 8.7)

I just tested this and saw it working on an 8.6 installation (with a fake config value of <= 8.5.0)

Crediting @martin107 for his patches in #2916603: Admin Form - Provide a link to a valid manifest.json file. which I just marked as a dupe.

Just running over the new form, standardizing it

1) Now using @inheritdoc where possible.

2) string translation.

Converted instances of t() into $this->t() which is the prefered form.
a #title string was untranslated.

3) drupal_set_message() is deprecated $this->messeneger->addWarning() is the future proof pattern

4) \Drupal is a antipattern -- which I have converted where possible

Drupal::currentUser() becomes $this->currentUser() etc

5) Directly accessing $_SERVER[] is discouraged. I have use the symphony standard of getting the same information from the request object.

Changed start_url to "/", also needed to re-roll

Thanks to both of you for the attention to this issue! I'm terrible at judging what is good and bad code in D8, but here are a few things I noticed:

1. +++ b/config/install/pwa.config.yml
   @@ -0,0 +1,8 @@
   +langcode:    'en'
   

   default config is missing the `display` property. Please set it to "browser" as the default.

2. +++ b/config/schema/pwa.schema.yml
   @@ -0,0 +1,26 @@
   +pwa.config:
   

   It seems as if the YML for pwa.config is duplicated here. Pardon my ignorance but can you explain why it appears twice?

3. +++ b/src/Form/ConfigurationForm.php
   @@ -0,0 +1,299 @@
   +        '4' => $this->t('browser'),
   

   The machine keys for display should be the untranslated words that appear in each t() to display a value. I'd also prefer to use capitalized words with spaces instead of dashes:

   • Fullscreen
   • Standalone
   • Minimal UI
   • Browser

4. +++ b/src/Form/ConfigurationForm.php
   @@ -0,0 +1,299 @@
   +      'file_validate_image_resolution' => ['512x512', '512x512'],
   

   Is the 512x512 here duplicated for a reason? If we want to validate, it would be good to also validate for 192x192 which is the only other icon size required by Lighthouse at the present.

5. +++ b/src/Form/ConfigurationForm.php
   @@ -0,0 +1,299 @@
   +  private function getDisplayValue($value, $needId) {
   

   If possible I'd like to ditch this function entirely and directly use strings as the stored values. If my desire is somehow violating a Drupal best-practice please just let me know :)

6. +++ b/src/Form/ConfigurationForm.php
   @@ -0,0 +1,299 @@
   +      $newSize = 192;
   

   ah I see now that you're transforming the 512x512. That's nice.

7. +++ b/src/manifest.php
   @@ -0,0 +1,134 @@
   +  public function get_output() {
   

   The output of this function will often be called and some sort of function caching would be good.

   Also, the application/json MIME type needs to be specified when this content is served. That's why we serve from PHP instead of generating the file and putting it in public FS.

8. +++ b/src/manifest.php
   @@ -0,0 +1,134 @@
   +          "gcm_sender_id": "103953800507"
   

   I hope this isn't a private key that someone accidentally checked in here!

   It should be removed form the patch either way.

One last thing, the core compatibility. I mentioned this in #9 and #15 and I really think we should be prepared to pull all of this back out once core commits their version. To ensure that it plays nice with a future version of core, let's defensively set the compat to <8.7 and keep bumping each core release until manifest is in core.

Looking at manifest.json -- get_output() and getCleanValues()

get_output() appears to be a re-implemenetation of json_encode.

looking at

http://php.net/manual/en/function.json-encode.php

This is a standard way of converting a php array into something that will make a {} json encoded object.

$arr = array('a' => 1, 'b' => 2, 'c' => 3, 'd' => 4, 'e' => 5);

echo json_encode($arr);

will output

{"a":1,"b":2,"c":3,"d":4,"e":5}

I hope this is constructive.

I am working on this.

Tiny update for #19 that adds the pwa.config route; otherwise installing the patched version will fail on account of the route not existing.

Original patch wasn't actually exposing the manifest.json route. My update fixes that and builds the manifest.json content from a PHP object instead of crafting a string, as per #21. It also deletes that private key, as per #20.

Sorry about this back and forth, one more change; actually expose the
element linking to the manifest.

(extremely moose's voice) This time for sure!

1. +++ b/config/install/pwa.config.yml
   @@ -0,0 +1,8 @@
   +site_name:   ''
   

   pwa.config.yml contain duplicate keys? is that right?

2. +++ b/config/schema/pwa.schema.yml
   @@ -0,0 +1,26 @@
   +pwa.config:
   +  type: config_object
   +  label: 'PWA config'
   +  mapping:
   +    site_name:
   +      type:   text
   +      label:  'Web app name'
   +    short_name:
   +      type:   text
   +      label:  'Short name'
   +    description:
   +      type:   text
   +      label:  'Description'
   +pwa.config:
   +  type: config_object
   +  label: 'PWA config'
   +  mapping:
   +    site_name:
   +      type:   text
   +      label:  'Web app name'
   +    short_name:
   +      type:   text
   +      label:  'Short name'
   +    description:
   ...
   +      label:  'Description'
   

   Also duplicated?

A) Removed duplicate keys / schema definitions.

B) Corrected 'langcode' key ... becomes 'lang'

C) I have converted the new class 'manifest' to become a drupal service called "pwa.manifest".

D) PWAController was extending ControllerBase .. a base class with lots of functionality, of which we were using nothing. So now instead of using \Drupal::state() which is an antipattern I am directly injecting it ( and the new pwa.manifest service)

E) The ConfigForm now has the pwa.manifest service injected.

F) Manifest now has all its dependencies directly injected.

G) The following code has been simplified, when pulling every config item into an array it can be vastly simplified by just calling $config_get->get();

 $input['site_name'] = $config_get->get('site_name');
 $input['short_name'] = $config_get->get('short_name');
 $input['background_color'] = $config_get->get('background_color');
 $input['theme_color'] = $config_get->get('theme_color');
 $input['image'] = $config_get->get('image');
 $input['display'] = $config_get->get('display');
 $input['default_image'] = $config_get->get('default_image');
 $input['image_small'] = $config_get->get('image_small');

H) Manifest now passes the coding standard review go a few name like delete_image [ now deleteImage ] have changed.

+++ b/src/Form/ConfigurationForm.php
@@ -0,0 +1,330 @@
+        $this->manifest->deleteImage();

$this->manifest is null here for me, and throw an error:

Error: Call to a member function deleteImage() on null in Drupal\pwa\Form\ConfigurationForm->submitForm()

This patch and interdiff does two things:
1) re-rolls previous patch #29
2) add textarea for specifying urls to precache
3) improved the form UI a bit

#31.2 is a very good idea.

This is just a minor tidy up.

t() is an anti-pattern... The function call comes from the global state.

$this->t() is within Drupal's namespace and so won't clash with Acme's version of t().

So I ask the question ... what next?

It seems to me that generating the manifest.json file can be regard as an expensive operation and the output needs to be cached, and invalidated upon changes to the associated config.

https://www.drupal.org/docs/8/api/cache-api/cache-tags

Sorry, I am having to carefully balance my time. I will try and get to this when I have more time.

@martin107 thanks for all the work you've done so far to bring this closer to completion! Don't worry about how long it takes. All your contribution time is appreciated!

In regards to caching, yes I agree that caching the server output of the function is needed. Sorry I didn't highlight enough code in #20 to make my comment obvious but my point #20.7 was in regards to caching the output of the manifest generation function:

The output of this function will often be called and some sort of function caching would be good.

Also, the application/json MIME type needs to be specified when this content is served. That's why we serve from PHP instead of generating the file and putting it in public FS.

Whoops, my bad! My comment about requiring PHP to generate manifest was inaccurate. That's how the actual Service Worker file is served, but for manifest.json in D7 we are generating manifest.json on cache clears and placing in public files. It gets replaced each cache clear but is otherwise low-overhead when it is requested.

https://cgit.drupalcode.org/pwa/tree/pwa.module?h=7.x-1.x#n302

Hey,

Just a small update to prevent a fatal error. The class name in services is written class: Drupal\pwa\manifest

No other changes.

Hi, I'm getting this error when I try to upload the manifest image and then save the config page:

Error: Call to a member function deleteImage() on null in pwa/src/Form/ConfigurationForm.php on line 256

with a white screen (The website encountered an unexpected error. Please try again later.)

When I go back to the /admin/config/system/pwa page, then no image is showing up. But it is uploaded into 'sites/default/files/pwa'.

Any idea how to fix this? On another site, with an older patch I had no issue.

Thanks!

I can get a little further along than Sahaj, I can actually upload an image and get it to display correctly. I do run into the same error he's reporting once I try to upload a different image though. (Using patch in #36)

I also got the same error as Sahaj with patch #36 but I found a workaround by saving the form first with the "Use the theme image" option checked then once saved I uploaded an image and it worked ok.

#39 works, but I am finding the manifest just disappears sometimes and I have to uninstall/reinstall the module to generate again. Working on a patch to do 4 simple things. Thanks for everyone's hard work so far.

1. Generate manifest.json in site root with variables we can set in Drupal admin/config.
2. Generate serviceworker.js in site root, with the ability to set variables from Drupal admin. Possibly using drupalSettings (currently SW is static and is only precaching assets based on an iframe hack in serviceworker-load.js).
3. Generate serviceworker-install.js. This code is all that's needed to register a SW https://developer.mozilla.org/en-US/docs/Web/API/ServiceWorkerContainer/... and so should be all that's in there Wordpress PWA currently registers two service workers under /wp-admin scope and root so so that could be an option later.
4. Include the js in the of every page. Link to manifest and include the two .js files.
5. Register /offline page tpl.

The SW will use workbox and allow for different caching strategies for different routes. https://developers.google.com/web/tools/workbox/modules/workbox-strategies. Stale while revalidate and Network First (Network Falling Back to Cache) to cache URLs on install so URLs are visible while offline.

This is being sponsored so I have time to work on this and will be doing so this week.
Will also use precache strategy to allow caching of static assets, js/css set via config.

I have a SW mostly finished to effectively turn a Drupal website into an "app," it will take values from Drupal config to replace URLs and use different cache strategies for various static assets depending on one's needs. Any feedback let me know I am going to start another thread to get this done. I think there is some code we can reuse from D8 such as generating the manifest and the offline page tpl but everything in the SW looks static and not up to best practices so will be redoing it in workbox.

Please look into the patch in #3060759: Fully working D8 version based on D7 Serviceworker which includes #36 and #39 from here, but updates to D8 module to where it should be, i.e. same service worker functionality as the D7 version.
Closing this issue to focus efforts.