Skip to content

fix: handle 0s transition duration in close animations and improve st… #64

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

Open
pontusarmini wants to merge 6 commits into
base: main
Choose a base branch
from
Open

fix: handle 0s transition duration in close animations and improve st… #64

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
75f1d15
fix: handle 0s transition duration in close animations and improve st…
pontusarmini Dec 17, 2025
11d96f0
fix: remove unused import
pattan Dec 17, 2025
e6abe73
chore: release v0.10.3-next.0
pattan Dec 17, 2025
0abdce6
added back the ad in the WP setup
erikarvidssonmadington Dec 18, 2025
219e7ae
fix(welcomepage): revert missing ad parameter
pattan Dec 18, 2025
92b7c31
chore: release v0.10.3-next.1
pattan Dec 18, 2025
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
3 changes: 3 additions & 0 deletions playground/local-dev/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ export default {
// downArrow: false,
// closeButtonText: "Lukk"
// },
options: {
closeButtonAnimationDuration: 2
},
setup: (wrapper, ad) => {
console.log(wrapper, ad);
return new Promise<void>((resolve, reject) => {
Expand Down
53 changes: 53 additions & 0 deletions playground/local-dev/welcomepage-test.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<title>WelcomePage Test (No Animation)</title>
<script src="https://cdn.tailwindcss.com"></script>
<style>
html {
overflow-x: hidden;
}
</style>
<script>
window.advantageCmdQueue = window.advantageCmdQueue || [];
</script>
<script type="module" src="./welcomepage-test.ts"></script>
</head>
<body>
<div class="container mx-auto px-4 py-4">
<h1 class="text-2xl font-bold mb-4">WelcomePage Test</h1>
<p class="mb-4">
This page tests the WelcomePage format with
<code>closeButtonAnimationDuration: 0</code>.
</p>
<p class="mb-4">
Click the "To [Site Title]" button in the ad to close it. It
should close immediately without getting stuck.
</p>

<!-- The wrapper that will load the WelcomePage ad -->
<advantage-wrapper>
<div slot="advantage-ad-slot">
<!-- Pointing to the existing welcomepage creative -->
<iframe
src="./welcomepage/welcomepage.html"
title="advantage-ad"
style="border: 0; width: 100vw; height: 100%"
scrolling="no"
></iframe>
</div>
</advantage-wrapper>

<div class="mt-8">
<p>Content below the ad...</p>
<p>
Lorem ipsum dolor sit amet, consectetur adipiscing elit. Sed
do eiusmod tempor incididunt ut labore et dolore magna
aliqua.
</p>
</div>
</div>
</body>
</html>
30 changes: 30 additions & 0 deletions playground/local-dev/welcomepage-test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import { Advantage } from "@src/advantage";
import { AdvantageFormatName } from "@src/types";
import logger from "@src/utils/logging";

const advantage = Advantage.getInstance();

advantage.configure({
formatIntegrations: [
{
format: AdvantageFormatName.WelcomePage,
options: {
// Set to 0 to test the fix for no-animation close
closeButtonAnimationDuration: 0,
autoCloseDuration: 0 // Disable auto-close for manual testing
},
setup: () => {
return new Promise<void>((resolve) => {
logger.debug("Setting up WelcomePage format integration");
resolve();
});
},
close: () => {
logger.debug("Closing WelcomePage format");
},
reset: () => {
logger.debug("Resetting WelcomePage format");
}
}
]
});
2 changes: 1 addition & 1 deletion src/advantage/formats/welcomepage.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
position: fixed;
z-index: var(--adv-wp-zindex);
opacity: 0;
transition: all 0.5s ease;
transition: all var(--adv-wp-transition-duration, 0.5s) ease;
transform: translateY(5%);
}

Expand Down
33 changes: 28 additions & 5 deletions src/advantage/formats/welcomepage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,12 @@ export const welcomePage: AdvantageFormat = {
return new Promise((resolve) => {
// Inser the CSS for the top scroll format
wrapper.insertCSS(varsCSS.concat(welcomepageCSS));
// Set the styles for the ad iframe
if (ad) {
setDimensionsUntilAdvantageAdSlot(ad);

if (config.closeButtonAnimationDuration !== undefined) {
wrapper.style.setProperty(
"--adv-wp-transition-duration",
`${config.closeButtonAnimationDuration}s`
);
}
Comment on lines 29 to 38
Copy link

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

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

The call to setDimensionsUntilAdvantageAdSlot(ad) was removed from the setup function. All other formats (topscroll.ts line 51, midscroll.ts line 15, multi-midscroll-base.ts line 69) call this function during setup, and welcomepage.ts still calls resetDimensionsUntilAdvantageAdSlot(ad) in its reset function (line 126). This removal appears unintentional and could break the ad dimensions setup for the WelcomePage format. The function call should be restored.

Copilot uses AI. Check for mistakes.

// Change the content of the UI layer
Expand Down Expand Up @@ -123,8 +126,11 @@ export const welcomePage: AdvantageFormat = {
resetDimensionsUntilAdvantageAdSlot(ad);
}
wrapper.resetCSS();
wrapper.style.removeProperty("--adv-wp-transition-duration");
},
close: (wrapper) => {
const container = wrapper.shadowRoot?.getElementById("container");

function handleTransitionEnd() {
wrapper.style.display = "none";
// Remove the event listener after it has been executed
Expand All @@ -134,8 +140,25 @@ export const welcomePage: AdvantageFormat = {
);
}

const container = wrapper.shadowRoot?.getElementById("container");
container?.addEventListener("transitionend", handleTransitionEnd);
if (container) {
const computedStyle = window.getComputedStyle(container);
const transitionProperty = computedStyle.transitionProperty;
const transitionDuration = computedStyle.transitionDuration;

const hasTransition =
transitionProperty !== "none" &&
transitionDuration.split(",").some((d) => parseFloat(d) > 0);

if (!hasTransition) {
wrapper.style.display = "none";
wrapper.classList.remove("show");
wrapper.style.height = "0px";
return;
}
Comment on lines +142 to +156
Copy link

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

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

The transition detection logic is duplicated between wrapper.ts (lines 467-475) and welcomepage.ts (lines 144-150). This creates a maintenance burden where bugs or improvements must be applied in multiple places. Consider extracting this logic into a shared utility function that both can use.

Copilot uses AI. Check for mistakes.

container.addEventListener("transitionend", handleTransitionEnd);
}

Comment on lines +142 to +160
Copy link

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

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

When the container is not found (null or undefined), the function silently continues to execute lines 162-163, which set wrapper styles. This could lead to unexpected behavior where the wrapper's height is set to 0px and the "show" class is removed even though no container was found. Consider returning early or handling the null case explicitly, similar to how the hasTransition check returns early on line 156.

Copilot uses AI. Check for mistakes.
wrapper.classList.remove("show");
wrapper.style.height = "0px";
}
Expand Down
22 changes: 22 additions & 0 deletions src/advantage/wrapper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -463,6 +463,27 @@ export class AdvantageWrapper extends HTMLElement implements IAdvantageWrapper {

animateClose(callback?: () => void) {
this.classList.add("animate");

const computedStyle = window.getComputedStyle(this);
const transitionProperty = computedStyle.transitionProperty;
const transitionDuration = computedStyle.transitionDuration;

// Check if there are any active transitions
// We need to handle multiple values (e.g. "0.5s, 1s") and check if any are > 0
const hasTransition =
transitionProperty !== "none" &&
transitionDuration.split(",").some((d) => parseFloat(d) > 0);

if (!hasTransition) {
this.classList.remove("animate");
this.style.height = "0px";
this.style.display = "none";
Comment on lines +477 to +480
Copy link

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

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

In the immediate cleanup path (no transition), the code sets style.height = "0px" before style.display = "none". This could cause a brief visual flicker if the element is still visible when height is set to 0. Consider setting display = "none" first, or remove the height setting since it's redundant when display is none. The transitioned path (line 502) sets height after setting up the listener, which is correct for animation.

Copilot uses AI. Check for mistakes.
if (callback) {
callback();
}
return;
}

this.addEventListener(
"transitionend",
() => {
Expand All @@ -471,6 +492,7 @@ export class AdvantageWrapper extends HTMLElement implements IAdvantageWrapper {
return;
}
this.style.display = "none";
this.classList.remove("animate");
if (callback) {
callback();
}
Expand Down