-
Notifications
You must be signed in to change notification settings - Fork 44
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
Added manifest.json, works with no navbar for PWA #6640
Added manifest.json, works with no navbar for PWA #6640
Conversation
It doesn't escape notice that installable key needs to be set to true for development, but needs to be set back to false to push (let's discuss how we handle pushing to frick/frack, since we would need it set back to true.) |
I see zero problem with setting it just to installable now. Seems not worth the back and forth later on, unless I'm missing something |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just realised that it is still in draft, so probably something might not work as expected. But left already a lot of comments regarded to code style etc. Besides,
- I am not able to X out from this prompt even though if I add app to the home screen.
- On Iphone font is not loaded correctly.
- There are differences in the implementations vs designs on both platforms
...ommonwealth/client/scripts/views/components/AddToHomeScreenPrompt/AddToHomeScreenPrompt.scss
Outdated
Show resolved
Hide resolved
...ommonwealth/client/scripts/views/components/AddToHomeScreenPrompt/AddToHomeScreenPrompt.scss
Outdated
Show resolved
Hide resolved
...ommonwealth/client/scripts/views/components/AddToHomeScreenPrompt/AddToHomeScreenPrompt.scss
Outdated
Show resolved
Hide resolved
...ommonwealth/client/scripts/views/components/AddToHomeScreenPrompt/AddToHomeScreenPrompt.scss
Outdated
Show resolved
Hide resolved
...commonwealth/client/scripts/views/components/AddToHomeScreenPrompt/AddToHomeScreenPrompt.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...commonwealth/client/scripts/views/components/AddToHomeScreenPrompt/AddToHomeScreenPrompt.tsx
Outdated
Show resolved
Hide resolved
...ommonwealth/client/scripts/views/components/AddToHomeScreenPrompt/AddToHomeScreenPrompt.scss
Outdated
Show resolved
Hide resolved
...commonwealth/client/scripts/views/components/AddToHomeScreenPrompt/AddToHomeScreenPrompt.tsx
Outdated
Show resolved
Hide resolved
...commonwealth/client/scripts/views/components/AddToHomeScreenPrompt/AddToHomeScreenPrompt.tsx
Outdated
Show resolved
Hide resolved
...commonwealth/client/scripts/views/components/AddToHomeScreenPrompt/AddToHomeScreenPrompt.tsx
Outdated
Show resolved
Hide resolved
...commonwealth/client/scripts/views/components/AddToHomeScreenPrompt/AddToHomeScreenPrompt.tsx
Outdated
Show resolved
Hide resolved
...ommonwealth/client/scripts/views/components/AddToHomeScreenPrompt/AddToHomeScreenPrompt.scss
Outdated
Show resolved
Hide resolved
@masvelio so i just tested this out, what happens is when we enter the responsive mode on chrome when already in console mode, the window.navigator.userData matches with Android causing the AndroidPrompt to be shown. I'm not sure how you're getting the IOSPrompt (unless you clicked on iPhone in the size in the console). This is the only way I've seen to reliably detect the OS for the PWA. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did another round of testing, but it is either on localhost using http or ngrok using https. Both are not perfect. Showing videos below from real android device (sorry for quality).
- http localhost - our popup appears but I clicking cancel or install does nothing
Screenrecorder-2024-02-21-14-22-53-453.mp4
- https ngrok - our popup appears and after clicking install I can see native popup. However, when I click cancel on native, I am not able to perform action again, neither on cancel, nor on install.
Screenrecorder-2024-02-21-14-26-23-359.2.mp4
As you can see it is very wonky. My proposition is that you resolve all my comments regarding the code and we should deploy it to demo/frick/frack and ask other engineers or @ilijabojanovic to test it out. It looks like we might have quite a lot of weird cases here so we need a plan how we want to go about it as probably we don't want to scare away our potential PWA users within first 2 minutes of using the app (or even before installing it). Also we need some strategy from now on how we want to test mobile features because this is already my third run on this PR and each of it was more than one hour of testing and doing code review cc @ForestMars @jnaviask @CowMuon @MuonShot
...s/commonwealth/client/scripts/views/components/AddToHomeScreenPrompt/IOSPrompt/IOSPrompt.tsx
Outdated
Show resolved
Hide resolved
.../commonwealth/client/scripts/views/components/AddToHomeScreenPrompt/IOSPrompt/IOSPrompt.scss
Outdated
Show resolved
Hide resolved
...wealth/client/scripts/views/components/AddToHomeScreenPrompt/AndroidPrompt/AndroidPrompt.tsx
Outdated
Show resolved
Hide resolved
...ealth/client/scripts/views/components/AddToHomeScreenPrompt/AndroidPrompt/AndroidPrompt.scss
Outdated
Show resolved
Hide resolved
...wealth/client/scripts/views/components/AddToHomeScreenPrompt/AndroidPrompt/AndroidPrompt.tsx
Outdated
Show resolved
Hide resolved
...s/commonwealth/client/scripts/views/components/AddToHomeScreenPrompt/IOSPrompt/IOSPrompt.tsx
Outdated
Show resolved
Hide resolved
...commonwealth/client/scripts/views/components/AddToHomeScreenPrompt/AddToHomeScreenPrompt.tsx
Outdated
Show resolved
Hide resolved
...commonwealth/client/scripts/views/components/AddToHomeScreenPrompt/AddToHomeScreenPrompt.tsx
Outdated
Show resolved
Hide resolved
@masvelio I was able to fix 2. since that was mainly due to the cancel button breaking in the last commit when I split the component and now install and cancel native prompts. The only wonky thing I see on my end (on Android) is when i install, then delete the app and then try re-installing it. If i do it within a couple seconds the install button doesn't work. I'm testing on https://frick.commonwealth.im/ This is likely due to the fact that the browser thinks the PWA is still installed, when its not. If I close and create a new tab after this the install and cancel buttons work well. We currently don't have a viable way to check if the app is installed on the user's phone or not navigator.getInstalledApps doesn't seem to work/gives an empty array. So not sure what solution i should go with for this |
…on-for-pwa-standalone-mode
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clicked around on android and iphone and after many fixes it works fine for me, so thank you @mw2000 for your hard work!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It works, just a quick code quality fix.
…://github.com/hicommonwealth/commonwealth into 6226-add-manifestjson-for-pwa-standalone-mode
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to go
Adds PWA support to the commonwealth mobile website, with a standalone application
Link to Issue
Closes: #6226
Description of Changes
"How We Fixed It"
Had to find where to place manifest.json file, but everything else was present in the issue.
For prompt, was able to build the custom component for iOS and android and check app status if it was run on browser or as PWA.
Test Plan
Deployment Plan