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: set default theme_color #426

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

debs-obrien
Copy link
Contributor

In order to get full scores on lighthouse we need to have the theme_color defined. At the moment it is set as undefined which means this will not be added to the head meta and therefore an error will show and you will not get full pwa score out of the box. Solution is to set the default color to white and allow users to modify it to another color if needed.

Screenshot 2021-01-21 at 13 32 24

The theme color only works for android devices and setting it will not actually do anything on ios devices but not setting it will cause an error and will not give full pwa score

https://web.dev/themed-omnibox/?utm_source=lighthouse&utm_medium=devtools

Lighthouse 7 is soon to be released and there are major changes to pwa scoring but theme_color is still the only one that is failing so this will fix not only for Lighthouse 6 but also for Lighthouse 7

https://github.com/GoogleChrome/lighthouse/releases

cc @pi0

@pi0
Copy link
Member

pi0 commented Jan 21, 2021

We was once using loading bar color as default but it was making design issues so had to revert. Have to test #ffff value on an android device to see how works with dark theme. If it changes behavior, we may use a warning instead that users have to set it (or set to null)

Also would be nice if can lint markdown in another pr :P (no worries btw)

@debs-obrien
Copy link
Contributor Author

ok sounds good. I don't own an android device so can't test it but at least a warning so that they add a color would be good rather than only knowing it from running lighthouse.

Yeah sorry my editor decided to lint everything. Managed to get rid of js/ts lint differences but markdown seemed to decide to stay 😬.

Base automatically changed from master to main January 26, 2021 21:59
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.

2 participants