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

fix: add clear: both to global ads #918

Merged
merged 1 commit into from
Nov 7, 2024
Merged

fix: add clear: both to global ads #918

merged 1 commit into from
Nov 7, 2024

Conversation

laurelfulford
Copy link
Contributor

While we're accounting for this in our ad insertion logic, sometimes ads end up a little too close to floated images and end up getting stuck behind them. This can happen with smaller paragraphs paired with taller images.

This PR adds a universal clear: both to the .newspack_global_ad class.

Steps to test

The easiest way to test this might be to use some trickery:

  1. Set up a post with at least one floated image, and a bunch of text in very short paragraphs.
  2. On the front end, if the in-story ad is not placed near the image, edit the post in the element inspector to move the image closer to the ad.
  3. Confirm that the ad sits below the image rather than beside it.

Before this fix:

CleanShot 2024-10-25 at 11 14 59

After this fix:

CleanShot 2024-10-25 at 11 15 22

@kmwilkerson flagging for you in case there's any issues with these styles (like if there's a case where ads should respect and flow around floated content).

@dkoo
Copy link
Contributor

dkoo commented Nov 6, 2024

@laurelfulford ready to merge?

@dkoo dkoo merged commit 8f951b5 into trunk Nov 7, 2024
8 checks passed
@dkoo dkoo deleted the fix/clear-floats branch November 7, 2024 20:03
matticbot pushed a commit that referenced this pull request Nov 15, 2024
## [3.2.2-alpha.1](v3.2.1...v3.2.2-alpha.1) (2024-11-15)

### Bug Fixes

* add clear: both to global ads ([#918](#918)) ([8f951b5](8f951b5))
@matticbot
Copy link
Contributor

🎉 This PR is included in version 3.2.2-alpha.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

matticbot pushed a commit that referenced this pull request Nov 25, 2024
## [3.2.2](v3.2.1...v3.2.2) (2024-11-25)

### Bug Fixes

* add clear: both to global ads ([#918](#918)) ([8f951b5](8f951b5))
@matticbot
Copy link
Contributor

🎉 This PR is included in version 3.2.2 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants