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.

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

Implement #181 - web app manifest support #182

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Implement #181 - web app manifest support #182

wants to merge 3 commits into from

Conversation

bhagany
Copy link
Contributor

@bhagany bhagany commented Mar 26, 2017

This feature will allow sites built with Perun to be installed as an app on systems that support it. When paired with #180, this will be a slick offline experience.

This was a bit more work to implement properly than I thought at first, primarily because I wanted to support automatically handling image files for the icons that are referenced in manifest.json. This required a refactor for images-resize, which is backwards-compatible. This in turn caused another small backwards-compatible addition to content-task to allow a boot tmp directory to be passed. This was necessary to accommodate tasks that want to write directly to tmp directories from within their pod, rather than passing data back to the main pod, as is the case for the image-handling tasks.

Other changes here are small improvements like pruning unused requires, and consistently handling the out-dir parameter to tasks. These should be self-explanatory, but if you have questions let me know.

I've also added tests for manifest.

@bhagany
Copy link
Contributor Author

bhagany commented Mar 26, 2017

I should also mention my sources for how to work with a manifest. The spec is here: https://w3c.github.io/manifest/, and this was useful for seeing how it's actually supported in browsers: https://developer.mozilla.org/en-US/Add-ons/WebExtensions/manifest.json. I paid the most attention to the output from http://realfavicongenerator.net/, which does a nice job of exposing the most relevant parts of a manifest. It seems to focus primarily on Android, preferring a different approach for other operating systems, hence the docstring for manifest. I plan on exploring this further, but this PR is a good base to build from.

src/io/perun.clj Outdated
:pod pod}))))

(deftask manifest
"Creates a manifest.json for Android (currently)"
Copy link
Member

Choose a reason for hiding this comment

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

why it's only for android? Wouldn't it work also on the desktop? What makes it Android specific?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I definitely don't understand everything about this, so I may be completely wrong. But for what it's worth - I took my cue from the Real Favicon Generator (http://realfavicongenerator.net/), which only references icons specifically for Android in manifest.json. I also can't find any examples of how manifest.json is supported on the desktop, and according to https://developer.mozilla.org/en-US/docs/Web/Manifest, no desktop browsers support it.

new-meta (write-file options tgt-path file resized-buffered-image resolution)
dimensions {:width (first new-dimensions) :height (second new-dimensions)}]
(merge file new-meta dimensions (select-keys options [:out-dir]))))
(def img-cache (atom {}))
Copy link
Member

Choose a reason for hiding this comment

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

oh, that is nice

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Glad you like it :)

@bhagany bhagany changed the base branch from master to images-resize-refactor December 30, 2017 05:20
This feature will allow sites built with Perun to be installed as an
app on systems that support it. When paired with #180, this will be
a slick offline experience.
@bhagany
Copy link
Contributor Author

bhagany commented Dec 30, 2017

I've split this PR into a few more parts (#203 and #205) so that they can be considered individually. I will also be updating this PR soon to improve functionality, and address the comments.

The Lighthouse Chrome extension made some recommendations for
attributes to include, so I've updated Perun's manifest.json
handling
@bhagany
Copy link
Contributor Author

bhagany commented Dec 30, 2017

@podviaznikov I've made a few improvements, and I believe I've addressed your comment on the docstring (you were right). Let me know if you'd like to see other changes.

@arichiardi
Copy link
Collaborator

This is a great feature!

Copy link
Member

@podviaznikov podviaznikov left a comment

Choose a reason for hiding this comment

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

Good job!
If you will have time, please update one of our example projects to use it. If you don't have time, merge away and create a ticket in the repo (to update example project with this task). Maybe someone else would be able to contribute it

@bhagany bhagany changed the base branch from images-resize-refactor to master January 9, 2018 13:26
@allentiak
Copy link
Contributor

Hi, @bhagany

Thanks again for your hard work!
Are you still working on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants