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

Carousel: Fix image styling in no-js format #6065

Merged
merged 2 commits into from
Oct 19, 2020

Conversation

Scotchester
Copy link
Contributor

Prior to this change, the item-visual-wrapper was adding some space under the image.

Tested this in Chrome, Firefox 44 (with #5974) and IE 11 and emulated 9. All behave as desired.

Changes

  • Carousel item visual wrapper's display changed from inline-block to block

Screenshots

Before

ino-js carousel with bad spacing

After

no-js carousel with fixed spacing

Checklist

  • PR has an informative and human-readable title
    • PR titles are used to generate the change log in releases; good ones make that easier to scan.
    • Consider prefixing, e.g., "Mega Menu: fix layout bug", or "Docs: Update Docker installation instructions".
  • Changes are limited to a single goal (no scope creep)
  • Code follows the standards laid out in the CFPB development guidelines

Prior to this change, the item-visual-wrapper was adding some space 
under the image.
@Scotchester Scotchester requested a review from a team October 2, 2020 20:05
@anselmbradford
Copy link
Member

What're the steps to see the before state? I tried disabling JS on the live site in Chrome and Safari and I don't see a gap.

@Scotchester
Copy link
Contributor Author

What're the steps to see the before state? I tried disabling JS on the live site in Chrome and Safari and I don't see a gap.

I guess Webkit has fixed the issue, but you can verify by disabling JavaScript in Firefox or visiting with IE 9.

@Scotchester Scotchester merged commit 5ec0f4f into main Oct 19, 2020
@Scotchester Scotchester deleted the fix-carousel-no-js-styling branch October 19, 2020 14:55
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.

2 participants