Skip to content

USWDS - Banner: Remove invalid baseline styling from banner button#5587

Merged
thisisdano merged 1 commit into
uswds:developfrom
aduth:aduth-rm-invalid-vertical-align
Dec 6, 2023
Merged

USWDS - Banner: Remove invalid baseline styling from banner button#5587
thisisdano merged 1 commit into
uswds:developfrom
aduth:aduth-rm-invalid-vertical-align

Conversation

@aduth

@aduth aduth commented Oct 30, 2023

Copy link
Copy Markdown
Contributor

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.

@mejiaj mejiaj left a comment

Copy link
Copy Markdown
Contributor

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.

@amyleadem amyleadem left a comment

Copy link
Copy Markdown
Contributor

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

@thisisdano thisisdano left a comment

Copy link
Copy Markdown
Contributor

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