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

feat: PWA Additions #3896

Merged

Conversation

Choromanski
Copy link
Contributor

@Choromanski Choromanski commented Jul 15, 2024

What type of PR is this?

  • bug (Warnings in Chrome Dev Tools)
  • feature

What this PR does / why we need it:

This PR adds PWA Manifest members to make the application more useable across different platforms.

Special notes for your reviewer:

Open to and encourage feedback on all manifest members updated - I've documented the changes I've made and provided reference links.

I would also like to implement the Richer Install UI but I'm going to need screenshots from both desktop and mobile if you could point me to any screenshots you have that would be great.
Note: All mobile screenshots need to be the same dimensions and all desktop screenshots need to be the name dimensions

Testing

Built docker image and was able to test functionality of all new members with the exception of display_override
See change screen shots below.

Changes

Shortcut Icons:

Resized them and was required to have one that was 96x96 to resolve icon warnings in Chrome Dev Tools
Before:
image

Now:
image

Changed icons to PNG so that the icons show up on desktop - I also changed them to #E58325 for contrast
Before:
image
After:
image

scope: "/"

Sets the scope of the application

dir: "auto"

Sets the text direction - Set to auto to allow support of both right to left and left to right
Note: Since the lang is set to "en" this could probably be set to "ltr" but I can't verify that.

id: "mealie"

Unique identifier for the PWA

display_override; ["standalone","minimal-ui","browser","window-controls-overlay"]

Sets display priority

prefer_related_applications: false

If applications in the related_applications section should be preferred over PWA.
Currently no official non-web applications exist but if one is developed in the future is should be listed in related_applications and this should be set to true

handle_links: "preferred":

In-scope links should be opened within the installed PWA

orientation: "any"

Default orientation of the application

categories: ["food"]

Categories the application belongs to

launch_handler: {"client_mode": ["focus-existing", "auto"]}

How the application is launched

edge_side_panel: {"preferred_width": 400}

The preferred width when launching the PWA in an Edge side panel

image

@Choromanski Choromanski marked this pull request as ready for review July 15, 2024 23:43
Copy link
Collaborator

@Kuchenpirat Kuchenpirat left a comment

Choose a reason for hiding this comment

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

Hey thanks for the work on this 😊

Before we can get to reviewing this.
Your Indentation setting is set to a different value than two spaces. Please use this for all code regarding mealie.

See the changes in frontend/nuxt.config.js as example

@Choromanski
Copy link
Contributor Author

Hey thanks for the work on this 😊

Before we can get to reviewing this. Your Indentation setting is set to a different value than two spaces. Please use this for all code regarding mealie.

See the changes in frontend/nuxt.config.js as example

I resolved the spacing issue, sorry about that.

@Kuchenpirat
Copy link
Collaborator

Thanks for the quick fix.
Looks all pretty good and your documentation makes is pretty easy to understand what is going on 👍

One thing. If you are already in there, would you be able to update the mealie icons to no longer include the white background? That would make it more consistent with the rest of the icons.

@Choromanski
Copy link
Contributor Author

I just went ahead and added some screenshots I made to resolve 2 of the warnings in the dev console:

Before:
image

After:
image

This also lets us use the Richer Install UI:

Desktop:
image

Mobile:
image

@Choromanski
Copy link
Contributor Author

Thanks for the quick fix. Looks all pretty good and your documentation makes is pretty easy to understand what is going on 👍

One thing. If you are already in there, would you be able to update the mealie icons to no longer include the white background? That would make it more consistent with the rest of the icons.

Do we have the original vector image anywhere? I tried working on the PNG for an hour and couldn't make it look good with my below average photo editing skills.

@Kuchenpirat
Copy link
Collaborator

Do we have the original vector image anywhere? I tried working on the PNG for an hour and couldn't make it look good with my below average photo editing skills.

oh i thought there was an svg version of that in the frontend, but apparently i was wrong and there is just the white background version. I have posted the svg code bellow, feel free to come back to me if you need anything more.

<svg style="width:100px;height:100px" viewBox="0 0 24 24">
    <path fill="currentColor" d="M8.1,13.34L3.91,9.16C2.35,7.59 2.35,5.06 3.91,3.5L10.93,10.5L8.1,13.34M13.41,13L20.29,19.88L18.88,21.29L12,14.41L5.12,21.29L3.71,19.88L13.36,10.22L13.16,10C12.38,9.23 12.38,7.97 13.16,7.19L17.5,2.82L18.43,3.74L15.19,7L16.15,7.94L19.39,4.69L20.31,5.61L17.06,8.85L18,9.81L21.26,6.56L22.18,7.5L17.81,11.84C17.03,12.62 15.77,12.62 15,11.84L14.78,11.64L13.41,13Z" />
</svg>

@Choromanski
Copy link
Contributor Author

Choromanski commented Jul 17, 2024

Do we have the original vector image anywhere? I tried working on the PNG for an hour and couldn't make it look good with my below average photo editing skills.

oh i thought there was an svg version of that in the frontend, but apparently i was wrong and there is just the white background version. I have posted the svg code bellow, feel free to come back to me if you need anything more.

<svg style="width:100px;height:100px" viewBox="0 0 24 24">
    <path fill="currentColor" d="M8.1,13.34L3.91,9.16C2.35,7.59 2.35,5.06 3.91,3.5L10.93,10.5L8.1,13.34M13.41,13L20.29,19.88L18.88,21.29L12,14.41L5.12,21.29L3.71,19.88L13.36,10.22L13.16,10C12.38,9.23 12.38,7.97 13.16,7.19L17.5,2.82L18.43,3.74L15.19,7L16.15,7.94L19.39,4.69L20.31,5.61L17.06,8.85L18,9.81L21.26,6.56L22.18,7.5L17.81,11.84C17.03,12.62 15.77,12.62 15,11.84L14.78,11.64L13.41,13Z" />
</svg>

Just pushed them, let me know what you think.

The icons on the previous photos seemed to have been a little off center - I believe I fixed this as well.

@Choromanski Choromanski requested a review from Kuchenpirat July 17, 2024 23:44
@Kuchenpirat
Copy link
Collaborator

Kuchenpirat commented Jul 22, 2024

Sorry it took me so long to come back to this.
I have found that if i reinstall the pwa on windows again the shortcuts are no longer added. I don't see a reason to why that would be, but maybe you could take a look at that.

NEW (Shortcuts missing) OLD (with Shortcuts)
image image

@Choromanski
Copy link
Contributor Author

I am still seeing all the shortcuts:
image

I know that pwa manifests are generally only rechecked by the browser once every 24 hours or so.

Could you try running it on a different port (just to force the browser to recheck the manifest) and reinstalling the PWA?

@Kuchenpirat
Copy link
Collaborator

Thanks for the information. 👍

This might be a me issue then. I did try installing via a different port with the same result.
I will do some investigating tomorrow and try a different machine to see if i can reproduce it somewhere else.

@Kuchenpirat
Copy link
Collaborator

I tried again today with two different machines, with different browsers and launching nuxt on different ports with the same results. Now i am out of computers and none the wiser why it would show up for you and not for me 🧐

@Choromanski
Copy link
Contributor Author

I tried again today with two different machines, with different browsers and launching nuxt on different ports with the same results. Now i am out of computers and none the wiser why it would show up for you and not for me 🧐

Do you have any errors in Chrome Dev Tools under Application > Manifest?

PWAs require HTTPS but I've noticed HTTP works if you use localhost. Is it possible you are not using localhost and missing HTTPS?

Copy link
Collaborator

@Kuchenpirat Kuchenpirat left a comment

Choose a reason for hiding this comment

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

Ok, i figured it out and it was pretty much me being uninformed. I actually did not install as PWA but as an chrome app or however those things are called.
Long story short. Everything shows up correctly including the shortcuts.

Thanks for your work on this and helping me with the review! 🚀

@Kuchenpirat Kuchenpirat merged commit 946b79b into mealie-recipes:mealie-next Jul 23, 2024
11 checks passed
@Choromanski
Copy link
Contributor Author

Ok, i figured it out and it was pretty much me being uninformed. I actually did not install as PWA but as an chrome app or however those things are called.
Long story short. Everything shows up correctly including the shortcuts.

Thanks for your work on this and helping me with the review! 🚀

Glad I could help!

@Choromanski Choromanski deleted the feature/pwa-refresh branch July 23, 2024 15:06
@Choromanski Choromanski restored the feature/pwa-refresh branch July 23, 2024 15:07
name: "Mealie",
short_name: "Mealie",
id: "mealie",

Choose a reason for hiding this comment

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

FWIW this defaults to the PWA's host at time of installation. Using a fixed value in the manifest will prevent users from installing potentially multiple different Mealie instances. Not that this is a common use case but worth mentioning nontheless.

https://developer.chrome.com/docs/capabilities/pwa-manifest-id

@felixschndr
Copy link

image

Just a quick question: How did you change the color of your interface from orange to pink?

@Choromanski
Copy link
Contributor Author

You have to update the THEME variables: https://docs.mealie.io/documentation/getting-started/installation/backend-config/

@Choromanski Choromanski deleted the feature/pwa-refresh branch October 23, 2024 02:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants