Uh oh!
There was an error while loading. Please reload this page.
progressive web app changes for lighthouse 100% score - #5926
Conversation
facebook-github-bot
commented
Nov 30, 2018
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed. If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks! |
facebook-github-bot
commented
Nov 30, 2018
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
iansu
commented
Nov 30, 2018
This looks great. Can you post the lighthouse score before these changes for comparison? |
ianschmitz
commented
Nov 30, 2018
Do you mind running the images through tiny png? |
dstroot
commented
Nov 30, 2018
I already ran PNG Crush on the images but happy to run them through tiny png to see what else we can squeeze out. Also happy to get a “before” image of the lighthouse score. It will take me a day or two. |
dstroot
commented
Dec 1, 2018
OK, I ran tiny png on the images. I missed the typescript template since I don't use it but it's fixed now. Files:
Lighthouse Report 3.pdf |
This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs. |
| <noscript> | ||
| You need to enable JavaScript to run this app. | ||
| </noscript> | ||
| <main> |
There was a problem hiding this comment.
Is it possible to put #root on main instead of having an extra DOM node?
There was a problem hiding this comment.
it's not a good idea to use main here. According to W3C spec we should have only one main per document. CRA should not have an opinion on what's the main content of the page.
| <noscript> | ||
| You need to enable JavaScript to run this app. | ||
| </noscript> | ||
| <main> |
There was a problem hiding this comment.
Is it possible to put #root on main instead of having an extra DOM node?
There was a problem hiding this comment.
it's not a good idea to use main here. According to W3C spec we should have only one main per document. CRA should have an opinion on what's the main content of the page.
| @@ -0,0 +1,2 @@ | |||
| User-agent: * | |||
There was a problem hiding this comment.
I'd put a comment link as the very first line
mrmckeb
commented
Jul 24, 2019
Hi @dstroot, I'm sorry it's been a long time. Are you still interested in getting this in? If so, let me know and we can try to get it into the next release. |
dscanlan
commented
Aug 5, 2019
If this is open to be completed. I can take a look? |
mrmckeb
commented
Aug 5, 2019
dstroot
commented
Aug 6, 2019
Sorry all for the delay in responding. I'd be happy if @dscanlan picks this up. I am in a new role and I don't have the cycles to take this over the finish line at this time. |
The project has done great work to create a progressive web app - however if you scaffold an app and deploy it "as is" you score low on the lighthouse test from Google. Why not make it awesome from the start! I know people have to change these files to make the app their own, but we should give them the best possible start. (of course they need to enable the service worker too).
I needed to:
manifest.jsonfilerobots.txtfileindex.htmlfile to add<meta name="description" content="Web site created using create-react-app">That's it! BOOM 100%!