Skip to content

Reintroduce the stickybox on wide screens #604

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

Merged

Conversation

davidfischer
Copy link
Contributor

Alternates between using the stickybox placement and the footer placement on unknown themes. It also adds an ID to the stickybox placement so we can see performance of it.

As written, this introduces a random element which may not be desirable. We could instead introduce the stickybox only on very wide screens (say >=1600px) or we could base it on the title of the project or something like that to make it deterministic. I'm open to suggestions.

    // TODO: Check this placement on the dashboard,
    // and see how this is performing.

The footer placement is performing very well at ~4x the CTR with ~50% the view rate vs. previously.

Ref: #562

@davidfischer davidfischer requested review from humitos and a team as code owners June 20, 2025 20:56
@davidfischer davidfischer requested a review from agjohnson June 20, 2025 20:56
Copy link
Member

@humitos humitos left a comment

Choose a reason for hiding this comment

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

Looks good to me. I tested it locally and it worked great!

Copy link
Contributor

@agjohnson agjohnson left a comment

Choose a reason for hiding this comment

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

Looks good 👍

@@ -198,11 +198,22 @@ export class EthicalAdsAddon extends AddonBase {
if (elementToAppend) {
elementToAppend.append(placement);
}
} else if (window.innerWidth > 1300 && Math.random() > 0.5) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This could be another place to eventually use media query events, so that we can re-evaluate this logic if the view port size changes. This will just execute once on page load.

Noting for later, I don't think we are using this type of logic anywhere else yet.

@@ -198,11 +198,22 @@ export class EthicalAdsAddon extends AddonBase {
if (elementToAppend) {
elementToAppend.append(placement);
}
} else if (window.innerWidth > 1300 && Math.random() > 0.5) {
Copy link
Member

Choose a reason for hiding this comment

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

I'd like to avoid the randomness here. I think a better approach is using the same logic for each user, so it doesn't change randomly on refresh.

@ericholscher
Copy link
Member

Will merge this for release today.

@ericholscher ericholscher merged commit bc62388 into main Jun 24, 2025
4 checks passed
@ericholscher ericholscher deleted the davidfischer/reintroduce-stickybox-on-widescreens branch June 24, 2025 10:20
@ericholscher
Copy link
Member

Did a simple version here: #607

ericholscher added a commit that referenced this pull request Jun 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants