-
Notifications
You must be signed in to change notification settings - Fork 16
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
Add a test page for cosmetic autoconsent rules #112
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Found some nits, other than that LGTM 👍
features/autoconsent/banner.html
Outdated
<head> | ||
<meta charset="utf-8"> | ||
<meta name="viewport" content="width=device-width"> | ||
<title>Autoconsent Cosmetic Test</title> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be in line with how we call it on the index page so "Cookie consent popup test".
index.html
Outdated
<li><a href="./features/autoconsent/">Cookie consent</a></li> | ||
<li><a href="./features/autoconsent/banner.html">Cookie consent banners</a></li> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, I wonder if we should rename previous one to be sth like "Cookie consent popups" and this one to be "Cookie consent banners" or notifications?
features/autoconsent/banner.js
Outdated
}, 500); | ||
|
||
window.results = { | ||
page: 'autoconsent', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's use unique id's for pages, this one can be 'autoconsent-banner'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you!
No description provided.