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

MWPW-141022 [Project PEP] Prompt Dismissal + Tie-in with App Launcher UX #2392

Merged
merged 65 commits into from
Jul 8, 2024
Merged
Show file tree
Hide file tree
Changes from 56 commits
Commits
Show all changes
65 commits
Select commit Hold shift + click to select a range
6b9a0bd
Added a way to mock entitlements in non prod environments for testing…
sharmrj May 21, 2024
9e95af6
Merge branch 'stage' of https://github.com/adobecom/milo into bypassXlg
sharmrj May 21, 2024
2ec9eff
Added a pulsing animation after dismissing the PEP modal
sharmrj May 22, 2024
601929d
Merge branch 'stage' of https://github.com/adobecom/milo into bypassXlg
sharmrj May 22, 2024
c41777b
Added the ability to debug pep in prod; added skipPepEntitlements option
sharmrj May 22, 2024
15f1339
Merge branch 'bypassXlg' of https://github.com/sharmrj/milo into dism…
sharmrj May 22, 2024
5b4c5e8
Merge branch 'stage' of https://github.com/adobecom/milo into dismiss…
sharmrj May 22, 2024
f5df88d
fixed url for loading the pep dismissal animation
sharmrj May 22, 2024
332c265
Another url fix for the pep dismissal animation css file
sharmrj May 22, 2024
538f5d3
slowed down the default animation
sharmrj May 22, 2024
565e1f9
Added the tooltip
sharmrj May 23, 2024
4ab6a2f
re-enabled tracking dismissed prompts
sharmrj May 23, 2024
e9a2143
Merge branch 'stage' of https://github.com/adobecom/milo into dismiss…
sharmrj May 23, 2024
b29f5f4
added tooltip that uses data attributes; cleaned up ring animation sl…
sharmrj May 27, 2024
bb9d6ef
Merge branch 'stage' of https://github.com/adobecom/milo into dismiss…
sharmrj May 27, 2024
fe91f40
styled the tooltip in accordance with the new figma spec sheet
sharmrj May 29, 2024
38043a9
Changed the color of the tooltip
sharmrj May 29, 2024
2941bf9
Pick up pep dismissal action config from section metadata; fixed lowe…
sharmrj May 31, 2024
7777616
Updated unit tests
sharmrj May 31, 2024
006a656
Merge branch 'stage' of https://github.com/adobecom/milo into dismiss…
sharmrj May 31, 2024
24820f9
updated an incomplete pep test
sharmrj May 31, 2024
114e6e1
Fixed dismissal actions firing when redirecting
sharmrj May 31, 2024
7d305c7
Removed a comment
sharmrj May 31, 2024
470a6db
added tests for running dismissal actions
sharmrj May 31, 2024
f4a0192
Tooltip and animation are now cleared if the app switcher is clicked
sharmrj May 31, 2024
a635600
added a missing semicolon
sharmrj May 31, 2024
52b18b1
small refactor of remove animation/tooltip on click logic
sharmrj May 31, 2024
aefd866
A bit of cleanup
sharmrj Jun 3, 2024
663641c
Merge branch 'stage' of https://github.com/adobecom/milo into dismiss…
sharmrj Jun 3, 2024
cda1018
removed some unnecessary code
sharmrj Jun 3, 2024
3e0a806
Fixed a bug introduced by one of the previous fixes
sharmrj Jun 3, 2024
ffb6a1f
fixed a classname
sharmrj Jun 3, 2024
baf08e6
Formatting of an html string
sharmrj Jun 3, 2024
4449fed
Merge branch 'stage' of https://github.com/adobecom/milo into dismiss…
sharmrj Jun 5, 2024
b927473
replaced all usage of right in absolute positioning with left
sharmrj Jun 5, 2024
0378d4f
Added CONFIG default values to dismissal action function parameter lists
sharmrj Jun 5, 2024
a95f74b
renamed a variable
sharmrj Jun 5, 2024
7077764
Removed an unused variable
sharmrj Jun 5, 2024
fb3468b
Grouped together common css rules in tooltip.css
sharmrj Jun 5, 2024
d5d6cd9
Combined dismissal css with the general webapp-prompt css
sharmrj Jun 5, 2024
319d276
Removed an unused import
sharmrj Jun 5, 2024
160c166
Merge branch 'stage' of https://github.com/adobecom/milo into dismiss…
sharmrj Jun 12, 2024
b727d52
Added a unit test
sharmrj Jun 12, 2024
9606978
Removed some unneeded newlines
sharmrj Jun 12, 2024
e2c99c3
Moved the tooltip down slightly
sharmrj Jun 14, 2024
2fd0567
Merge branch 'stage' of https://github.com/adobecom/milo into dismiss…
sharmrj Jun 14, 2024
15e7363
Merge branch 'stage' of https://github.com/adobecom/milo into dismiss…
sharmrj Jun 18, 2024
c640167
Added a missing curly brace
sharmrj Jun 18, 2024
538c9eb
added a missing semicolon
sharmrj Jun 18, 2024
afcd4ad
made the dismissal config its own entity
sharmrj Jun 20, 2024
2343413
added some variables to the focus animation css
sharmrj Jun 20, 2024
32504b2
removed a redundant style
sharmrj Jun 20, 2024
99c5ec7
cleaned up the padding css for the tooltip
sharmrj Jun 20, 2024
d4b68ec
minor refactoring
sharmrj Jun 20, 2024
8d3e25a
Changed the tooltip fontfamily to use the milo styles variable define…
sharmrj Jun 20, 2024
b2b0577
Removed an unnecessary css rule
sharmrj Jun 20, 2024
fb508bd
Merge branch 'stage' of https://github.com/adobecom/milo into dismiss…
sharmrj Jun 27, 2024
a995f25
Replaced tabs with spaces in webapp-prompt.css
sharmrj Jun 28, 2024
d47915d
Fixed the PEP unit tests
sharmrj Jul 2, 2024
e2c89f7
Merge branch 'stage' of https://github.com/adobecom/milo into dismiss…
sharmrj Jul 2, 2024
1f64afb
clock cleanup in tests
sharmrj Jul 2, 2024
d8348e1
fixed an issue with the redirect
sharmrj Jul 8, 2024
f332e8a
small change
sharmrj Jul 8, 2024
1e196b3
Fixed eslint error by making a method static
sharmrj Jul 8, 2024
e2068f6
Fixed failing tests
sharmrj Jul 8, 2024
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
115 changes: 115 additions & 0 deletions libs/features/webapp-prompt/webapp-prompt.css
Original file line number Diff line number Diff line change
Expand Up @@ -188,3 +188,118 @@
}
}
}

/* DISMISSAL TOOLTIP */

[data-pep-dismissal-tooltip]::after {
content: attr(data-pep-dismissal-tooltip);
display: inline-flex;
z-index: 3;
height: fit-content;
width: 8.875rem;
top: 125%;
left: -300%;
word-break: break-word;
salonijain3 marked this conversation as resolved.
Show resolved Hide resolved
border-radius: 7px;

padding-inline: 0.5625rem;
padding-block: 0.25rem 0.3125rem;

font-family: var(--body-font-family);
font-size: 0.75rem;
line-height: 0.9375rem;
color: white;
}

[data-pep-dismissal-tooltip]::before {
content: '';
z-index: 2;
width: 0.44rem;
height: 0.44rem;
border-radius: 0.05rem;
left: calc(50% - 0.22rem);
top: 115%;

transform: rotate(45deg);
}

[data-pep-dismissal-tooltip]::before,
[data-pep-dismissal-tooltip]::after {
background-color: #3B63FB;
Copy link
Contributor

Choose a reason for hiding this comment

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

This is already defined in the individual :after / :before definitions, does it need to be defined here as well?

position: absolute;
pointer-events: none;
transition: opacity 0.5s;
}

@media (min-width: 1520px) {
[data-pep-dismissal-tooltip]::after {
left: calc(50% - 5rem);
}
}

/* DISMISSAL ANIMATION */

.coach-indicator {
--coach-indicator-ring-default-color: rgba(56,146,243);
--coach-indicator-ring-diameter: 1.25rem;
--coach-indicator-ring-border-size: 2px;
--coach-indicator-ring-inline-size: var(--coach-indicator-ring-diameter);
--coach-indicator-ring-block-size: var(--coach-indicator-ring-diameter);
--coach-indicator-first-ring-delay-fraction: 0;
--coach-indicator-second-ring-delay-fraction: 0.33;
--coach-indicator-third-ring-delay-fraction: 0.66;
--animation-duration: 3000ms;
}

@keyframes pulse {
0% {
transform: scale(0.8);
opacity: 0;
}

50% {
transform: scale(1.5);
opacity: 1;
}

100% {
transform: scale(2);
opacity: 0;
}
}

.coach-indicator .coach-indicator-ring {
display: block;
position: absolute;
top: 14%;
left: 13%;

border-style: solid;
border-width: var(--coach-indicator-ring-border-size);
border-color: var(--coach-indicator-ring-default-color);

inline-size: var(--coach-indicator-ring-inline-size);
block-size: var(--coach-indicator-ring-block-size);
animation: pulse var(--animation-duration) linear;
animation-fill-mode: both;

border-radius: 5px;
}

.coach-indicator .coach-indicator-ring:nth-child(1) {
animation-delay: calc(var(--animation-duration)*var(--coach-indicator-first-ring-delay-fraction));
}

.coach-indicator .coach-indicator-ring:nth-child(2) {
animation-delay: calc(var(--animation-duration)*var(--coach-indicator-second-ring-delay-fraction));
}

.coach-indicator .coach-indicator-ring:nth-child(3) {
animation-delay: calc(var(--animation-duration)*var(--coach-indicator-third-ring-delay-fraction));
}

@media (prefers-reduced-motion: reduce) {
.coach-indicator .coach-indicator-ring {
animation: none;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Are the newlines on purpose on the lines: 205, 210, 211, 227, 244, 279, 283, 288?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also there is a mix of tabs and spaces. All should be spaces.

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've fixed both the issues (some of the newlines were on purposes; I replaced each tab with the appropriate number of spaces)

83 changes: 78 additions & 5 deletions libs/features/webapp-prompt/webapp-prompt.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,21 @@
import { getConfig, decorateSVG } from '../../utils/utils.js';
import { replaceKey, replaceText } from '../placeholders.js';

export const DISMISSAL_CONFIG = {
animationCount: 2,
animationDuration: 2500,
tooltipMessage: 'Use the App Switcher to quickly find apps.',
tooltipDuration: 5000,
};

const CONFIG = {
selectors: { prompt: '.appPrompt' },
delay: 7000,
loaderColor: '#EB1000',
...DISMISSAL_CONFIG,
};

const getElemText = (elem) => elem?.textContent?.trim().toLowerCase();
const getElemText = (elem) => elem?.textContent?.trim();

const getMetadata = (el) => [...el.childNodes].reduce((acc, row) => {
if (row.children?.length === 2) {
Expand All @@ -35,6 +43,54 @@
return icons.company;
};

const showTooltip = (
element,
message = CONFIG.tooltipMessage,
time = CONFIG.tooltipDuration,
) => {
element.setAttribute('data-pep-dismissal-tooltip', message);
const cleanup = () => element.removeAttribute('data-pep-dismissal-tooltip');
const timeoutID = setTimeout(cleanup, time);
element.addEventListener('click', () => {
cleanup();
clearTimeout(timeoutID);
}, { once: true });
};

const playFocusAnimation = (
element,
iterationCount = CONFIG.animationCount,
animationDuration = CONFIG.animationDuration,
) => {
element.classList.add('coach-indicator');
element.style.setProperty('--animation-duration', `${animationDuration}ms`);
const rings = [];
const createRing = () => toFragment`
<div
class="coach-indicator-ring"
style="animation-iteration-count: ${iterationCount};">
</div>`;
for (let i = 0; i < 3; i += 1) {
const ring = createRing();
element.insertAdjacentElement('afterbegin', ring);
rings.push(ring);
}
// The cleanup function is added to the event queue
// some time after the end of the animation because
// the cleanup isn't high priority but it should be done
// eventually. (Animation truly ends slightly after
// animationDuration * iterationCount due to animation-delay)
const cleanup = () => {
rings.forEach((ring) => ring.remove());
element.classList.remove('coach-indicator');
};
const timeoutID = setTimeout(cleanup, (iterationCount + 1) * animationDuration);
element.addEventListener('click', () => {
cleanup();
clearTimeout(timeoutID);
}, { once: true });
};

const modalsActive = () => !!document.querySelector('.dialog-modal');

const waitForClosedModalsThen = (loadPEP) => {
Expand Down Expand Up @@ -162,6 +218,10 @@
const metadata = getMetadata(content.querySelector('.section-metadata'));
metadata['loader-duration'] = parseInt(metadata['loader-duration'] || CONFIG.delay, 10);
metadata['loader-color'] = metadata['loader-color'] || CONFIG.loaderColor;
metadata['dismissal-animation-count'] = parseInt(metadata['dismissal-animation-count'] ?? CONFIG.animationCount, 10);
metadata['dismissal-animation-duration'] = parseInt(metadata['dismissal-animation-duration'] ?? CONFIG.animationDuration, 10);
metadata['dismissal-tooltip-message'] ??= CONFIG.tooltipMessage;
metadata['dismissal-tooltip-duration'] = parseInt(metadata['dismissal-tooltip-duration'] ?? CONFIG.tooltipDuration, 10);
this.options = metadata;
};

Expand All @@ -181,7 +241,7 @@
: '';

return toFragment`<div
daa-state="true" daa-im="true" daa-lh="PEP Modal_${this.options['product-name']}"
daa-state="true" daa-im="true" daa-lh="PEP Modal_${this.options['product-name']?.toLowerCase()}"
class="appPrompt" style="margin: 0 ${this.offset}px">
${this.elements.closeIcon}
<div class="appPrompt-icon">
Expand All @@ -200,7 +260,7 @@
};

addEventListeners = () => {
this.anchor?.addEventListener('click', this.close);
this.anchor?.addEventListener('click', () => this.close({ dismissalActions: false }));
document.addEventListener('keydown', this.handleKeyDown);

[this.elements.closeIcon, this.elements.cta]
Expand All @@ -212,7 +272,7 @@
};

initRedirect = () => setTimeout(() => {
this.close({ saveDismissal: false });
this.close({ saveDismissal: false, dismissalActions: false });

Check warning on line 275 in libs/features/webapp-prompt/webapp-prompt.js

View check run for this annotation

Codecov / codecov/patch

libs/features/webapp-prompt/webapp-prompt.js#L275

Added line #L275 was not covered by tests
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you test this by defining a hash as the redirect URL? That would avoid the page reload, but would trigger the UI changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately this still results in this line ostensibly never being run. It's difficult to say why, but it may have something to do with how fakeTimers interacts with the event loop.n

window.location.assign(this.options['redirect-url']);
}, this.options['loader-duration']);

Expand All @@ -224,14 +284,27 @@
document.cookie = `dismissedAppPrompts=${JSON.stringify([...dismissedPrompts])};path=/`;
};

close = ({ saveDismissal = true } = {}) => {
close = ({ saveDismissal = true, dismissalActions = true } = {}) => {
const appPromptElem = document.querySelector(CONFIG.selectors.prompt);
appPromptElem?.remove();
clearTimeout(this.redirectFn);
if (saveDismissal) this.setDismissedPrompt();
document.removeEventListener('keydown', this.handleKeyDown);
this.anchor?.focus();
this.anchor?.removeEventListener('click', this.close);

if (dismissalActions) {
playFocusAnimation(
this.anchor,
this.options['dismissal-animation-count'],
this.options['dismissal-animation-duration'],
);
showTooltip(
this.anchor,
this.options['dismissal-tooltip-message'],
this.options['dismissal-tooltip-duration'],
);
}
};

static getDismissedPrompts = () => {
Expand Down
27 changes: 26 additions & 1 deletion test/features/webapp-prompt/mocks/pep-prompt-content.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,13 @@
export default ({ color, loaderDuration, redirectUrl, productName }) => `<div>
export default ({
color,
loaderDuration,
redirectUrl,
productName,
animationCount,
animationDuration,
tooltipMessage,
tooltipDuration,
}) => `<div>
<p>
<picture>
<source type="image/webp" srcset="http://localhost:2000/test/features/webapp-prompt/mocks/media-icon.png" media="(min-width: 600px)">
Expand Down Expand Up @@ -27,5 +36,21 @@ export default ({ color, loaderDuration, redirectUrl, productName }) => `<div>
<div>product-name</div>
<div>${productName}</div>
</div>`}
${animationCount && `<div>
<div>dismissal-animation-count</div>
<div>${animationCount}</div>
</div>`}
${animationDuration && `<div>
<div>dismissal-animation-duration</div>
<div>${animationDuration}</div>
</div>`}
${tooltipMessage && `<div>
<div>dismissal-tooltip-message</div>
<div>${tooltipMessage}</div>
</div>`}
${tooltipDuration && `<div>
<div>dismissal-tooltip-duration</div>
<div>${tooltipDuration}</div>
</div>`}
</div>
</div>`;
5 changes: 4 additions & 1 deletion test/features/webapp-prompt/test-utilities.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { setViewport } from '@web/test-runner-commands';
import sinon from 'sinon';
import init from '../../../libs/features/webapp-prompt/webapp-prompt.js';
import init, { DISMISSAL_CONFIG } from '../../../libs/features/webapp-prompt/webapp-prompt.js';
import { viewports, mockRes as importedMockRes } from '../../blocks/global-navigation/test-utilities.js';
import { getConfig, loadStyle, setConfig, updateConfig } from '../../../libs/utils/utils.js';

Expand All @@ -17,13 +17,16 @@ export const allSelectors = {
progressWrapper: '.appPrompt-progressWrapper',
progress: '.appPrompt-progress',
appSwitcher: '#unav-app-switcher',
indicatorRing: '.coach-indicator-ring',
tooltip: '[data-pep-dismissal-tooltip]',
};

export const defaultConfig = {
color: '#b30b00',
loaderDuration: 7500,
redirectUrl: 'https://www.adobe.com/?pep=true',
productName: 'photoshop',
...DISMISSAL_CONFIG,
};

export const mockRes = importedMockRes;
Expand Down
Loading
Loading