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

Display sidebar ad when scrolled #7621

Merged
merged 2 commits into from
Oct 30, 2020

Conversation

davidfischer
Copy link
Contributor

There is a JS bug around ad display when linked directly to an anchor somewhat down the page. By default, we display an ad in the sidebar if it would be visible without scrolling the sidebar (who does that?) and put it in the footer otherwise. The math was slightly off on whether the bottom of the sidebar would be visible if the window was already scrolled.

Testing steps

  • You have to test this on a project that has a sidebar less than the full height of the page but the body has anchors that would require scrolling to reach.
  • Load the page directly to an anchor down the page, the ad should display it in the sidebar.

There is a JS bug around ad display when linked directly
to an anchor somewhat down the page.
By default, we display an ad in the sidebar if it would be
visible without scrolling the sidebar (who does that?)
and put it in the footer otherwise.
The math was slightly off on whether the bottom of the sidebar
would be visible if the window was already scrolled.
@davidfischer davidfischer requested a review from a team October 29, 2020 22:04
@davidfischer
Copy link
Contributor Author

The CI error is unrelated.

@stsewd
Copy link
Member

stsewd commented Oct 29, 2020

The CI error is unrelated.

Looks like you need to update your branch to match the changes from master

Copy link
Member

@ericholscher ericholscher 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 👍

@davidfischer davidfischer merged commit 5ff3c09 into master Oct 30, 2020
@davidfischer davidfischer deleted the davidfischer/ad-display-when-scrolled branch October 30, 2020 21:18
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.

3 participants