Skip to content

Conversation

aduth
Copy link
Contributor

@aduth aduth commented Oct 30, 2023

Summary

Removes invalid styles from the banner component. These styles had no effect and were safely removed.

Problem statement

As a developer, I expect that USWDS ships valid CSS, so that (a) it is correct, (b) my CSS bundles are optimized only to include what has a meaningful effect on styles, and (c) my code will not be flagged by automated validation tools.

The application of vertical-align: baseline is invalid here because the component is also styled as display: block

References:

Note that vertical-align only applies to inline, inline-block and table-cell elements: you can't use it to vertically align block-level elements.

Source: https://developer.mozilla.org/en-US/docs/Web/CSS/vertical-align

Applies to: | inline-level and table-cell elements

Source: https://drafts.csswg.org/css2/#propdef-vertical-align

Validation warning via VSCode:

Screenshot 2023-10-30 at 8 56 32 AM

Solution

Removes the vertical-align: baseline styles.

Copy link
Contributor

@mejiaj mejiaj left a comment

Choose a reason for hiding this comment

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

Good find, thanks for submitting.

No issues in any screen size with removing this. Tested on Develop preview branch.

Copy link
Contributor

@amyleadem amyleadem left a comment

Choose a reason for hiding this comment

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

Thank you for this fix! Everything looks good to me.

I checked the following items:

  • Confirmed that vertical-align is invalid on block elements
  • Confirmed that this change removes vertical-align from .usa-banner__button
  • Confirmed that there is no visual change in positioning for .usa-banner__button

Copy link
Contributor

@thisisdano thisisdano left a comment

Choose a reason for hiding this comment

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

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
From: GSA Role: Dev Development/engineering skills needed
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants