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

USWDS - Utilities: Update inaccurate comments #5859

Merged
merged 8 commits into from
Apr 26, 2024

Conversation

amyleadem
Copy link
Contributor

@amyleadem amyleadem commented Apr 10, 2024

Summary

Updated the code comments on utility Sass partials. These comments now reflect the correct utility class names and values.

Important

This PR builds on the changes added by @aduth in PR #5700. Thank you @aduth!

Breaking change

This is not a breaking change. This update only affects code comments.

Related issue

Closes #5860

Related pull requests

This PR builds on the changes from @aduth in PR #5700.

Preview link

N/A

Problem statement

The code comments on the files in the packages/uswds-utilities/src/styles/rules/ directory are outdated and inaccurate. These code comments should be updated so that developers can use the code comments as reference (especially for utilities not documented on our website).

Solution

This PR updates outdate utility class references to match uswds 3.0 naming conventions.

Testing and review

  1. Confirm that all files in the packages/uswds-utilities/src/styles/rules/ have comments that reflect current USWDS naming structures
  2. Confirm comments are formatted consistently

@amyleadem amyleadem changed the title USWDS - Utilities: Update comments USWDS - Utilities: Update inaccurate comments Apr 10, 2024
@amyleadem amyleadem marked this pull request as ready for review April 10, 2024 21:37
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 making these fixes. I was able to test comments are accurate and up-to-date.

Can we fix the closing } in the code examples?

packages/uswds-utilities/src/styles/rules/add-aspect.scss Outdated Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

Tip

Not alphabetical order, but it's output in uswds.css.

/* uswds.css */
.add-list-reset{
  margin-bottom:0;
  margin-top:0;
  padding-left:0;
  list-style:none;
}

packages/uswds-utilities/src/styles/rules/border.scss Outdated Show resolved Hide resolved
packages/uswds-utilities/src/styles/rules/border.scss Outdated Show resolved Hide resolved
@@ -3,10 +3,10 @@
overflow
----------------------------------------
usage:
.overflow-[modifier]
.overflow-[modifier]*-[key]
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the value of the *?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe it indicates that the modifier is optional. At least, that is my assumption based on the other comments with this format. I read it as the class can can be written as .overflow-hidden or .overflow-y-hidden.

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.

This is looking great, thank you for combing through these and updating.

Besides James' new line comment, I noticed a space missing for one value. Minuscule change but might as well while we're here 😄

packages/uswds-utilities/src/styles/rules/opacity.scss Outdated Show resolved Hide resolved
@amyleadem
Copy link
Contributor Author

@mejiaj and @mahoneycm Thanks for those comments. I've moved the closing braces in the comments to a new line to meet standards and added the missing space to the opacity comment. Please let me know if I missed anything!

Copy link
Member

@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.

Thanks @aduth and @amyleadem! A somewhat thankless but important task

@thisisdano
Copy link
Member

But now it's thankless no longer :)

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.

USWDS - Bug: Inaccurate utility code comments
5 participants