Visitar URL original
USWDS - Banner: Remove invalid baseline styling from banner button by aduth · Pull Request #5587 · uswds/uswds · GitHub
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

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.

@thisisdano thisisdano added Type: Enhancement From: GSA Originating feedback or use case came from GSA; provenance, not priority. Role: Dev Dev expertise is relevant. Project Active Role tracks ownership; this label is not an assignment. and removed Status: Triage labels Nov 2, 2023
@thisisdano thisisdano added this to the uswds 3.7.1 milestone Nov 2, 2023

@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 Originating feedback or use case came from GSA; provenance, not priority. Role: Dev Dev expertise is relevant. Project Active Role tracks ownership; this label is not an assignment.

Projects

No open projects
Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants