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.

Sign up for GitHub

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

Jump to bottom

PR: Optimise JS issue #325 #327

Merged
merged 26 commits into from
Mar 7, 2023
Merged

PR: Optimise JS issue #325 #327

merged 26 commits into from
Mar 7, 2023

Conversation

nelsonic
Copy link
Member

@nelsonic nelsonic commented Mar 5, 2023 •
edited
Loading

This PR adds a <button> to the inded.html and some JS code to only load the Flutter Web App if the person taps/clicks the button: #326 + #325

It takes us from a performance score of 70 to 100 using "smoke and mirrors".
Also addressed all A11y and SEO issues so now we have a perfect score on PageSpeed Insights:
https://pagespeed.web.dev/report?url=https%3A%2F%2Fapp.dwyl.com%2F

image

@LuchoTurtle / @SimonLab please review and merge when you can. :shipit:
Thanks. 👌

@nelsonic nelsonic added chore a tedious but necessary task often paying technical debt priority-2 Second highest priority, should be worked on as soon as the Priority-1 issues are finished technical A technical issue that requires understanding of the code, infrastructure or dependencies spike The simplest possible experiment to explore potential solutions to a problem flutter Flutter related issues research Research required; be specific tech-debt A feature/requirement implemented in a sub-optimal way & must be re-written labels Mar 5, 2023
@nelsonic
Copy link
Member Author

nelsonic commented Mar 5, 2023 •
edited
Loading

Cloudflare pages was deploying before the deploy.yml had run ...
image

This is obviously undesireable so I disabled it: #322 (comment)

nelsonic added 15 commits March 5, 2023 08:21
@nelsonic
temporarily run deploy.yml on "optimise-js-issue-#325" branch to test…
0f82a07 
… JS minification #326
@nelsonic
attempt to fix invalid workflow file https://github.com/dwyl/app/acti…
3e2e2a4 
…ons/runs/4335135865/workflow #326
@nelsonic
move package-lock.json to root of project to fix https://github.com/d…
995035b 
…wyl/app/actions/runs/4335157948/jobs/7569534788#step:9:11 #326
@nelsonic
run esbuild before gh-pages #326
4f54308
@nelsonic
pwd #326
f3dc065
@nelsonic
\n pwd #326
2cf24fe
@nelsonic
run pwd #326
283571c
@nelsonic
setTimeout for Smoke & Mirrors #326
57c9e6a
@nelsonic
inline loadScript #326
e34e15d
@nelsonic
git rm web/load.js
0fda38d
@nelsonic
delay for 1200ms #326
3157147
@nelsonic
ls -lath ./build/web #326
1a02c65
@nelsonic
chmod 777 ./build/web #326
e002231
@nelsonic
Delay JS for Lighthouse Bot #326 epic Smoke & Mirrors! 🙈
aef51a7
@nelsonic
create "I am not a robot" <button> for #326
f20ce41
@nelsonic
Copy link
Member Author

nelsonic commented Mar 5, 2023

image

Need to fix scale on iPhone 📱...

@nelsonic
Copy link
Member Author

nelsonic commented Mar 6, 2023

Ok, that was too much ... 😜

image

@nelsonic
Copy link
Member Author

nelsonic commented Mar 6, 2023

Better but not quite perfect ...

image

@nelsonic
Copy link
Member Author

nelsonic commented Mar 6, 2023

Much better:

image

@nelsonic nelsonic requested review from SimonLab and LuchoTurtle March 6, 2023 07:45
@nelsonic nelsonic added the awaiting-review An issue or pull request that needs to be reviewed label Mar 6, 2023
@nelsonic
Copy link
Member Author

nelsonic commented Mar 6, 2023

@SimonLab please review and merge when you can. Thanks 👌

@nelsonic nelsonic mentioned this pull request Mar 6, 2023
Open
1 task
LuchoTurtle
LuchoTurtle requested changes Mar 6, 2023
View reviewed changes
Copy link
Member

@LuchoTurtle LuchoTurtle left a comment

Choose a reason for hiding this comment

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

Looks great! Awesome!
I just think adding that small css cursor property makes sense 👍

web/index.html Outdated Show resolved Hide resolved
@nelsonic
Copy link
Member Author

nelsonic commented Mar 6, 2023

cursor: pointer added as per @LuchoTurtle's suggestion. ✅
Over to you @SimonLab 🙏

@nelsonic nelsonic added the T5m Quick tasks that take 5 mins or less. See: GTD 2 min rule. label Mar 7, 2023
SimonLab
SimonLab approved these changes Mar 7, 2023
View reviewed changes
Copy link
Member

@SimonLab SimonLab left a comment

Choose a reason for hiding this comment

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

Code looks good.
I'm still not sure about having a "loading page". I understand the reason but it feels counter-intuitive to use Flutter for mobile web. However I don't have any other solutions apart from using Flutter only for mobile "native" app and a web framework (Phoenix, ...) for the web version, so merging 👍

@SimonLab SimonLab merged commit e3a4554 into main Mar 7, 2023
@SimonLab SimonLab deleted the optimise-js-issue-#325 branch March 7, 2023 10:31
@nelsonic
Copy link
Member Author

nelsonic commented Mar 7, 2023

@SimonLab agreed. 👌
I dislike landing/loading pages a lot! 😢
But I see it as a necessary evil for now ... ⏳
It gives the person viewing the App something they can see while waiting for Flutter to load ...

I wish we had resources to maintain a dedicated web version of our App for fast loading and SEO. 🐎
But ... equally I love the fact that Flutter can build for various platforms off the same codebase. ❤️
So we can build a PWA that we can all use on both Mobile and Desktop for fast iteration. 📱 & 💻 = 🎉
And people trying our App don't have "install" anything if they don't want to, it just works!
That is game-changing for adoption rates.
Last year when I was doing FinTech work, one of the "Neo Banks" went down the iOS-only route
and it bit them really hard as they only had 10k registered accounts by the time I rolled off. 💭
The break-even point and all the MGMT projections for the app were 100k ... 💯
People couldn't even start the onboarding process without downloading the App!! 😞
This is a very poor strategy (IMO) in a world with so many Apps competing for attention.

The hardest part by far of any App is the UI/UX and if we can get it pixel-perfect in Flutter using M3
then we're not going to re-do it all in PETAL unless there is a compelling reason e.g: Perf, A11y or SEO.

Again, I agree. 👍
If we can figure out a way of avoiding the loading screen, please open an issue and we can remove it. 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LuchoTurtle LuchoTurtle LuchoTurtle approved these changes

@SimonLab SimonLab SimonLab approved these changes

Assignees

@SimonLab SimonLab

Labels
awaiting-review An issue or pull request that needs to be reviewed chore a tedious but necessary task often paying technical debt flutter Flutter related issues priority-2 Second highest priority, should be worked on as soon as the Priority-1 issues are finished research Research required; be specific spike The simplest possible experiment to explore potential solutions to a problem T5m Quick tasks that take 5 mins or less. See: GTD 2 min rule. tech-debt A feature/requirement implemented in a sub-optimal way & must be re-written technical A technical issue that requires understanding of the code, infrastructure or dependencies
Projects
dwyl app kanban
Status: ✅ Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nelsonic @SimonLab @LuchoTurtle