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.

Sign up for GitHub

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

Jump to bottom

fix: metrics consent prompt location and styling #353

Merged
merged 3 commits into from
Jan 4, 2023
Merged

fix: metrics consent prompt location and styling #353

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0463f99
fix: metrics consent prompmt location and styling
0xDanomite Dec 19, 2022
36fa6ab
fix: metrics consent styling updates per cr
0xDanomite Dec 21, 2022
f4a6d22
fix: 341 - scroll issues on mobile fix
0xDanomite Jan 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions src/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
<link href="styles.css?v=0.4" type="text/css" rel="stylesheet"/>
</head>
<body id="checker" class="sans-serif charcoal bg-snow-muted">
<div class="hidden js-metrics-notification-decline-warning metrics-notification-wrapper">
<div class="hidden js-metrics-notification-decline-warning metrics-notification-wrapper bg-navy bt bw1 border-aqua">
0xDanomite marked this conversation as resolved.
Show resolved Hide resolved
<div class="metrics-notification-container">
<span id="metrics-notification-decline-warning" class="metrics-notification-text">
We will limit collection of metrics to only necessary features, 'sessions' and 'views'. See <a href="https://support.count.ly/hc/en-us/articles/360037441932-Web-analytics-JavaScript-#features-for-consent">Countly's group_features</a> for more information.
Expand All @@ -23,12 +23,13 @@
</div>
</div>
</div>
<div class="hidden js-metrics-notification metrics-notification-wrapper">
<div class="hidden js-metrics-notification metrics-notification-wrapper bg-navy bt bw1 border-aqua">
0xDanomite marked this conversation as resolved.
Show resolved Hide resolved
<div class="metrics-notification-spacer"></div>
<div class="metrics-notification-container">
<span class="metrics-notification-text">We're collecting <a href="https://github.com/ipfs/ipfs-gui/issues/125">web-vitals, pageview, and other metrics</a> in order to improve and prioritize our work on IPFS and its public gateways.
Please consent to the collection of these metrics to assist in our efforts!</span>
<div class="metrics-notification-buttons">
<button id="metrics-notification-accept" class="js-metrics-notification-accept">☑&nbsp;Agree</button>
<button id="metrics-notification-accept" class="js-metrics-notification-accept">Allow</button>
<button id="metrics-notification-decline" class="js-metrics-notification-decline">Decline</button>
</div>
</div>
Expand Down
42 changes: 34 additions & 8 deletions src/styles.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,28 +131,41 @@ div#checker\.results .Node:nth-child(1) .Link {
* Should be a color that is not too bright, but still visible
*/
div.metrics-notification-wrapper {
background-color: rgba(246, 248, 251, 1);
justify-content: center;
padding: 15px 0;
margin: 0 auto;
display: flex;
width: 100%;
max-width: 1024px;
position: fixed;
bottom: 0;
left: 0;
z-index: 1;
Comment on lines +137 to +140
Copy link
Contributor

Choose a reason for hiding this comment

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

this should've fixed it :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would have thought so too! it should have, but firefox was having issue with scroll as you noticed so the results table container needed an overflow setting

}

div.metrics-notification-container {
width: 100%;
display: flex;
align-items: center;
justify-content: center;
max-width: 1024px;
Copy link
Contributor

Choose a reason for hiding this comment

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

prolly not a bad idea to define min-width too?

padding: 15px 0;
gap: 0.5rem 5rem;
}

.hidden {
display: none;
}

.hidden.metrics-notification-wrapper {
opacity: 0;
position: relative;
display: none;
}


div.metrics-notification-wrapper .metrics-notification-text {
color: rgba(7, 58, 83, 1);
order: 1;
color:rgba(246, 248, 251, 1);
order: 2;
align-self: flex-start;
text-align: left;
}

div.metrics-notification-wrapper .metrics-notification-buttons {
Expand All @@ -176,24 +189,34 @@ div.metrics-notification-wrapper button {
align-content: center;
vertical-align: center;
text-align: center;
border-radius: 5px;
font-weight: bold;
padding: 8px;
}

.metrics-notification-wrapper button#metrics-notification-accept {
border: 1pt solid rgba(107, 196, 206, 1);
background-color: rgba(107, 196, 206, 1);
color:rgba(246, 248, 251, 1);
}

button#metrics-notification-warning-close {
border: 1pt solid rgba(107, 196, 206, 1);
/* color: rgba(7, 58, 83, 1); */
}

.metrics-notification-spacer {
width: 70px;
}

@media (max-width: 1002px) {

.metrics-notification-wrapper .metrics-notification-text {
color: rgba(7, 58, 83, 1);
order: 1;
color:rgba(246, 248, 251, 1);
order: 2;
align-self: flex-start;
padding: 0 2rem;
text-align: left;
}

.metrics-notification-container {
Expand All @@ -219,6 +242,9 @@ button#metrics-notification-warning-close {
z-index: 99980;
cursor: pointer;
margin: 0 1em 0 0;
position: fixed;
bottom: 15px;
left: 15px;
0xDanomite marked this conversation as resolved.
Show resolved Hide resolved
}

.cookieConsentToggle:disabled {
Expand Down