Skip to content
This repository has been archived by the owner on Apr 5, 2024. It is now read-only.

trigger warning when browser dependent CDN was detected #518

Merged
merged 4 commits into from
Dec 7, 2021

Conversation

kota-yata
Copy link
Contributor

Display warning if the pasted domain is matched with the constant list (URL_LIST) in checkBrowserDepedency, which currently contains only fonts.googleapis.com.
Screen Shot 2021-12-03 at 0 20 37

@kota-yata kota-yata changed the title trigger warning when browser dependent CDN was detected #514 trigger warning when browser dependent CDN was detected Dec 3, 2021
@kota-yata
Copy link
Contributor Author

Related to #514

@kota-yata
Copy link
Contributor Author

Copy link
Contributor

@mozfreddyb mozfreddyb left a comment

Choose a reason for hiding this comment

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

Thanks for the quick pull request! Please switch to using the URL parsing API and we're good to go.

app.js Outdated Show resolved Hide resolved
style.css Outdated
@@ -192,6 +192,14 @@ footer {
background-color: #ffb6c1;
}

#warning {
background: #d5e47f;
Copy link
Contributor

Choose a reason for hiding this comment

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

Not a huge fan of the color, but I'm also not known for my good taste. Maybe something more like a warning-orange? :) It's OK to leave this if you feel strong.y

Copy link
Contributor Author

@kota-yata kota-yata Dec 6, 2021

Choose a reason for hiding this comment

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

I made it little more vivid in 8698f0d . Yellow looks better to me as a warning color than orange. Bright yellow is also better for accessibility.

Copy link
Contributor

@mozfreddyb mozfreddyb left a comment

Choose a reason for hiding this comment

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

Thanks! I found another bug, though. :)
We'll get more warning boxes, if you keep hitting the "hash" button.
You need to either remove the box unconditionally (before the if (isBrowserDependent)...) or better, modify in-place if it already exists.

P.S: Thank you for working on the color.

@kota-yata
Copy link
Contributor Author

Probably fixed in 3c8c796

@mozfreddyb mozfreddyb merged commit 1e2150c into mozilla:gh-pages Dec 7, 2021
@mozfreddyb
Copy link
Contributor

thank you!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants