Skip to content

New icons and new data-ampdevmode for Validator extension#30011

Merged
honeybadgerdontcare merged 7 commits intoampproject:masterfrom
honeybadgerdontcare:val-ext-icons
Aug 28, 2020
Merged

New icons and new data-ampdevmode for Validator extension#30011
honeybadgerdontcare merged 7 commits intoampproject:masterfrom
honeybadgerdontcare:val-ext-icons

Conversation

@honeybadgerdontcare
Copy link
Copy Markdown
Contributor

@honeybadgerdontcare honeybadgerdontcare commented Aug 27, 2020

fixes #24176

updates look of existing icons
introduces new dev mode icon when data-ampdevmode is on html tag
removes number of warnings from icon when page is passing validation (e.g. warnings only)
updates background color for failing validation

@amp-owners-bot
Copy link
Copy Markdown

amp-owners-bot bot commented Aug 27, 2020

Hey @ampproject/wg-caching! These files were changed:

validator/js/chromeextension/_locales/en/messages.json
validator/js/chromeextension/amp-link-128.png
validator/js/chromeextension/amp-link-38.png
validator/js/chromeextension/background.js
validator/js/chromeextension/dev-128.png
validator/js/chromeextension/dev-38.png
validator/js/chromeextension/invalid-128.png
validator/js/chromeextension/invalid-38.png
validator/js/chromeextension/manifest.json
validator/js/chromeextension/no-amp-128.png
validator/js/chromeextension/no-amp-38.png
validator/js/chromeextension/valid-128.png
+1 more

Copy link
Copy Markdown
Member

@westonruter westonruter left a comment

Choose a reason for hiding this comment

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

The updated logic works as expected!

However, I have some feedback about the icon changes. I had a difficult time seeing the icon badge associated with the AMP icon for everything other than the error state. I had to squint to see the 🔗 , ⚠️ , and ✅ badges.

As such, I think they need improvement for accessibility.

State Icon
Error
Dev Mode
Pass
Available

@honeybadgerdontcare
Copy link
Copy Markdown
Contributor Author

@Gregable please re-review, requested changes made

Copy link
Copy Markdown
Member

@Gregable Gregable left a comment

Choose a reason for hiding this comment

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

see comment in thread.

@honeybadgerdontcare honeybadgerdontcare merged commit edce2e8 into ampproject:master Aug 28, 2020
@honeybadgerdontcare honeybadgerdontcare deleted the val-ext-icons branch August 28, 2020 22:27
ed-bird pushed a commit to ed-bird/amphtml that referenced this pull request Dec 10, 2020
…#30011)

* new chrome extension icons

* add dev mode setting

* minor fixes, update version

* remove anti-pattern

* Elaborate isInDevMode condition in comment.

* remove badgetitle from handleamppass

* s/isInDevMode/onlyErrorIsDevMode/g
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update Validator Chrome extension to show debug icon instead of error when in dev mode

4 participants