Skip to content

USWDS - Utilities: Add "full-percent" palettes to top, left, right #5633

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Dec 6, 2023
Merged

USWDS - Utilities: Add "full-percent" palettes to top, left, right #5633

merged 1 commit into from
Dec 6, 2023

Conversation

amyleadem
Copy link
Contributor

@amyleadem amyleadem commented Nov 21, 2023 •
edited
Loading

Summary

Fixed a bug that prevented the CSS from generating .left-full, .right-full, and .top-full utility classes.

Breaking change

This is not a breaking change.

Related issue

Closes #5632

Related pull requests

Changelog PR

Problem statement

Our documentation for .left-, .right-, and .top- utility classes state that USWDS offers .left-full, .right-full, and .top-full classes. However, these do not appear in the default USWDS CSS. USWDS CSS does currently provide a .bottom-full class.

Solution

Added "full-percent" palettes to .top-, .left-, .right- utility classes.

Testing and review

  • Confirm that USWDS should include .left-full, .right-full, and .top-full classes.
  • Confirm that the generated CSS now includes .left-full, .right-full, and .top-full classes.
  • Confirm that the new classes generate the expected property values

@amyleadem amyleadem mentioned this pull request Nov 21, 2023
Closed
@amyleadem amyleadem marked this pull request as ready for review November 21, 2023 19:22
@mejiaj
Copy link
Contributor

mejiaj commented Nov 21, 2023

@amyleadem two questions;

  1. How does this affect compiled CSS size?
  2. Is this fixing a regression?

@amyleadem
Copy link
Contributor Author

@mejiaj

  1. Updating this adds 1KB to the CSS
    • Before: 652KB (30002 lines)
    • After: 653KB (30014 lines)
  2. It doesn't appear to be a regression.
    • The bottom property received "full-percent" 5 years ago in A bunch of little fixes #2504. I don't see any history of left, right, or top having "full-percent".
    • For reference, in uswds-site the .left-full class was added to the docs at about the same time 5 years ago.

mejiaj
mejiaj approved these changes Nov 21, 2023
View reviewed changes
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.

Thanks for addressing this

mahoneycm
mahoneycm approved these changes Nov 27, 2023
View reviewed changes
Copy link
Contributor

@mahoneycm mahoneycm left a comment

Choose a reason for hiding this comment

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

Works like a charm!

  • Pattern matches bottom-full which previously worked
  • Correctly adds top, left, and right relative positioning

@amyleadem amyleadem mentioned this pull request Dec 4, 2023
Merged
thisisdano
thisisdano approved these changes Dec 6, 2023
View reviewed changes
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.

Simple and effective

@thisisdano thisisdano merged commit ae48235 into develop Dec 6, 2023
@thisisdano thisisdano deleted the al-full-utility-class branch December 6, 2023 18:46
@amyleadem amyleadem mentioned this pull request Dec 6, 2023
Closed
@thisisdano thisisdano mentioned this pull request Dec 8, 2023
Merged
@SherfeyInv SherfeyInv mentioned this pull request Jun 7, 2024
Closed
@sennhann29 sennhann29 mentioned this pull request Jul 26, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
3 more reviewers

@mejiaj mejiaj mejiaj approved these changes

@thisisdano thisisdano thisisdano approved these changes

@mahoneycm mahoneycm mahoneycm approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

USWDS - Bug: "full" display position utility classes do not generate
4 participants
@amyleadem @mejiaj @thisisdano @mahoneycm