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

Mntor 1564 - Add Survey Recruitment Link #3030

Merged
merged 6 commits into from
May 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 4 additions & 2 deletions src/appConstants.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ const requiredEnvVars = [
]

const optionalEnvVars = [
'CANCEL_SUBSCRIPTION_FLOW',
'EMAIL_TEST_RECIPIENT',
'FX_REMOTE_SETTINGS_WRITER_PASS',
'FX_REMOTE_SETTINGS_WRITER_SERVER',
Expand All @@ -52,8 +53,9 @@ const optionalEnvVars = [
'HIBP_BREACH_DOMAIN_BLOCKLIST',
'LIVE_RELOAD',
'PORT',
'SENTRY_DSN_LEGACY',
'CANCEL_SUBSCRIPTION_FLOW'
'RECRUITMENT_BANNER_TEXT',
'RECRUITMENT_BANNER_LINK',
'SENTRY_DSN_LEGACY'
]

/** @type {Record<string, string>} */
Expand Down
12 changes: 8 additions & 4 deletions src/client/css/global.css
Original file line number Diff line number Diff line change
Expand Up @@ -104,16 +104,20 @@ body > header {
position: fixed;
top: 0;
width: 100%;
display: flex;
justify-content: space-between;
align-items: center;
background: white;
box-shadow: 0 6px 6px -8px #0000;
padding: var(--padding-md) var(--padding-lg);
transition: box-shadow 0.3s;
z-index: 2;
}

body > header .header-wrapper {
align-items: center;
display: flex;
justify-content: space-between;
padding: var(--padding-md) var(--padding-lg);
width: 100%;
}

.scrolled body > header {
box-shadow: 0 6px 6px -8px #000;
}
Expand Down
1 change: 1 addition & 0 deletions src/client/css/index.css
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@
@import './global.css';
@import './nav.css';
@import './userMenu.css';
@import './surveyBanner.css';
61 changes: 61 additions & 0 deletions src/client/css/surveyBanner.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

.recruitment-banner {
text-align: center;
position: relative;
padding: var(--padding-md) var(--padding-md);
background-color: var(--purple-50);
}

.recruitment-banner ul {
list-style-type: none;
}

#recruitment-banner-link {
color: white;
text-decoration: underline;
display: inline-block;
padding: 0 var(--padding-md);

/* Width of the dismiss button: */
padding-right: var(--padding-xl);
max-width: var(--max-width-p);
margin: 0 auto;
}

#recruitment-banner-link:hover {
cursor: pointer;
text-decoration: none;
}

.recruitment-banner .dismiss-btn {
position: absolute;
top: 0;
bottom: 0;
right: var(--padding-md);
margin: auto;
outline: none;
height: 32px;
width: 32px;
border-radius: 50%;
background-color: transparent;
border: 2px solid transparent;
padding: var(--padding-xs);
display: flex;
flex-direction: column;
justify-content: center;
align-items: center;
margin-left: var(--padding-xl);
cursor: pointer;
transition: border-color 0.3s ease-out;
}

.recruitment-banner .dismiss-btn:hover {
border-color: var(--purple-70);
}

.recruitment-banner .dismiss-btn .x-close-icon path {
fill: white;
}
1 change: 1 addition & 0 deletions src/client/js/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,4 @@ import './userMenu.js'
import './dialog.js'
import './utils.js'
import './analytics.js'
import './recruitmentSurvey.js'
35 changes: 35 additions & 0 deletions src/client/js/recruitmentSurvey.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

const recruitmentBanner = document.querySelector('.recruitment-banner')
const recruited = document.cookie.split('; ').some((item) => item.trim().startsWith('recruited-2023-05='))

function init () {
if (!recruitmentBanner) {
return
}

// Only show the banner and set event listeners if the user has not interacted with the survey
if (!recruited) {
recruitmentBanner.toggleAttribute('hidden')
const recruitmentBannerLink = document.getElementById('recruitment-banner-link')
const recruitmentDismissButton = document.getElementById('recruitment-banner-dimiss')
recruitmentBannerLink.addEventListener('click', handleEvent)
recruitmentDismissButton.addEventListener('click', handleEvent)
}
}

function handleEvent (e) {
const date = new Date()
date.setTime(date.getTime() + 30 * 24 * 60 * 60 * 1000)
document.cookie = 'recruited-2023-05=true; expires=' + date.toUTCString() + '; path=/; SameSite=Lax'

switch (true) {
case e.target.matches('#recruitment-banner-dimiss'):
document.getElementById('recruitment-banner-link').parentElement.remove()
Copy link
Contributor

Choose a reason for hiding this comment

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

Q: Do we need to remove the link/button event listeners here as well?

Copy link
Contributor Author

@maxxcrawford maxxcrawford May 16, 2023

Choose a reason for hiding this comment

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

I had the same question myself here: #3030 (comment) I'll ask UX and follow-up!

Nevermind. I misread your question. Looking into 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.

Welp, I'm quoting StackOverflow here:

In modern browsers, you do not need to remove event listeners before removing elements from the DOM. If there are no direct references to that particular DOM element elsewhere in your javascript (e.g. the DOM element reference stored in a JS variable), then the DOM element will be garbage collected after removal. The DOM garbage collector is smart enough to know that an event listener does not count as a reference to that DOM element once it's been removed from the DOM (because DOM events cannot happen when it is removed from the DOM).

I think it's parents' removal is enough to break the references.

Unless the references on L16/L17 are still "references" in memory. I was gonna let QA see what breaks from here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The only other thing I could do is directly remove the entire banner, rather than the .parentElement bit.

Suggested change
document.getElementById('recruitment-banner-link').parentElement.remove()
recruitmentBanner.remove()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Vinnl @pdehaan LMK if that is enough answer for y'all. 😅

break
}
}

init()
1 change: 1 addition & 0 deletions src/controllers/breaches.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ async function breachesPage (req, res) {
const data = {
breachesData,
breachLogos: req.app.locals.breachLogoMap,
countryCode: getCountryCode(req),
emailVerifiedCount,
emailTotalCount,
selectedEmailIndex,
Expand Down
14 changes: 8 additions & 6 deletions src/views/guestLayout.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,14 @@ const guestLayout = data => {
</head>
<body>
<header>
<a href='/'>
<img class='monitor-logo' srcset='/images/monitor-logo-transparent.webp 213w, /images/[email protected] 425w' width='213' height='33' alt='${getMessage('brand-fx-monitor')}'>
</a>
<menu>
<li><a href='/user/breaches' data-cta-id='sign-in-1' class='button secondary'>${getMessage('sign-in')}</a></li>
</menu>
<div class="header-wrapper">
<a href='/'>
<img class='monitor-logo' srcset='/images/monitor-logo-transparent.webp 213w, /images/[email protected] 425w' width='213' height='33' alt='${getMessage('brand-fx-monitor')}'>
</a>
<menu>
<li><a href='/user/breaches' data-cta-id='sign-in-1' class='button secondary'>${getMessage('sign-in')}</a></li>
</menu>
</div>
</header>
<main data-partial='${data.partial.name}'>
${data.partial(data)}
Expand Down
47 changes: 37 additions & 10 deletions src/views/mainLayout.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,29 @@ const mainLayout = data => {
const metaDescription = data.meta?.socialDescription ?? getMessage('meta-desc-2')
const pageUrl = `${AppConstants.SERVER_URL}${data.pathname ?? '/'}`

const showRecruitmentBanner = () => {
// Only show if ENVs are set, user language is set to "en" and user locale is set to the United States.
// Otherwise, return empty string
if (
AppConstants.RECRUITMENT_BANNER_TEXT &&
AppConstants.RECRUITMENT_BANNER_LINK &&
getLocale().includes('en') &&
data.countryCode === 'us'
) {
return recruitmentBannerMarkup()
}
return ''
}

const recruitmentBannerMarkup = () => `
<div class="recruitment-banner" hidden>
<a id="recruitment-banner-link" class="text-link" href="${AppConstants.RECRUITMENT_BANNER_LINK}" target="_blank" rel="noopener noreferrer" data-ga="send-ga-pings" data-event-category="Recruitment" data-event-label="${AppConstants.RECRUITMENT_BANNER_TEXT}">${AppConstants.RECRUITMENT_BANNER_TEXT}</a>
<button id="recruitment-banner-dimiss" class="dismiss-btn">
<svg aria-label="Close" class="x-close-icon" role="button" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 20 20"><path d="M14.348 14.849a1.2 1.2 0 0 1-1.697 0L10 11.819l-2.651 3.029a1.2 1.2 0 1 1-1.697-1.697l2.758-3.15-2.759-3.152a1.2 1.2 0 1 1 1.697-1.697L10 8.183l2.651-3.031a1.2 1.2 0 1 1 1.697 1.697l-2.758 3.152 2.758 3.15a1.2 1.2 0 0 1 0 1.698z"/></svg>
</button>
</div>
`

return `
<!doctype html>
<html lang=${getLocale()}>
Expand Down Expand Up @@ -52,16 +75,20 @@ const mainLayout = data => {
</head>
<body>
<header>
<a href='/user/breaches'>
<img class='monitor-logo' srcset='/images/monitor-logo-transparent.webp 213w, /images/[email protected] 425w' width='213' height='33' alt='${getMessage('brand-fx-monitor')}'>
</a>
<div class='nav-wrapper'>
<button class='nav-toggle'>
<svg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 10 8' width='20'>
<path d='M1 1h8M1 4h8M1 7h8' stroke='#000' stroke-width='1' stroke-linecap='round'/>
</svg>
</button>
${userMenu(data)}
${/* Cancel Premium Subscription */ ''}
${showRecruitmentBanner()}
<div class="header-wrapper">
<a href='/user/breaches'>
<img class='monitor-logo' srcset='/images/monitor-logo-transparent.webp 213w, /images/[email protected] 425w' width='213' height='33' alt='${getMessage('brand-fx-monitor')}'>
</a>
<div class='nav-wrapper'>
<button class='nav-toggle'>
<svg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 10 8' width='20'>
<path d='M1 1h8M1 4h8M1 7h8' stroke='#000' stroke-width='1' stroke-linecap='round'/>
</svg>
</button>
${userMenu(data)}
</div>
</div>
</header>

Expand Down