progressive web app changes for lighthouse 100% score #5926
Conversation
|
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! |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
|
This looks great. Can you post the lighthouse score before these changes for comparison? |
|
Do you mind running the images through tiny png? |
|
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. |
|
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
|
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. |
|
If this is open to be completed. I can take a look? |
|
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%!