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-Site - Card: Add header exdent to variant table #2228

Merged
merged 7 commits into from
Sep 20, 2023

Conversation

mahoneycm
Copy link
Contributor

@mahoneycm mahoneycm commented Aug 3, 2023

Summary

Adds missing header--exdent class to card variants table.

Related issue

Closes #2209

Preview link

Card variants →

Problem statement

The card variants table was missing the header--exdent variant despite the design system offering the class and styles.

Solution

Add usa-card__header--exdent variant to table.

Additional Context

Exdent classes were added in uswds/uswds#3437 (May 2020)

It looks to me like the header exdent was unintentionally left out since the class was added.

Testing and review

  1. View card guidance page
  2. Confirm header--exdent variant is listed
  3. Confrim guidance matches other exdent guidance
  4. Confirm its place in table makes sense

Testing checklist

  • usa-card__header--exdent variant present in table
  • Class name matches actual class in the design system
  • Variant guidance is appropriate and understandable
  • Variant is proper place on table
  • Approved changelog

@mahoneycm mahoneycm changed the title USWDS-Site: Add header exdent to variant table USWDS-Site - Card: Add header exdent to variant table Aug 3, 2023
@mahoneycm mahoneycm marked this pull request as ready for review August 3, 2023 16:53
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.

Thanks for fixing this! Added a couple of little suggestions for the changelog.

Also, wondering if the variants list should be alphabetized. It doesn't seem to be the standard on the site but it would make my little organization heart happy. No action needed here, just flagging it.

I checked the following:

  • usa-card__header--exdent variant present in table
  • Class name matches actual class in the design system
  • Variant guidance is appropriate and understandable
  • Variant is proper place on table
  • Approved changelog

_data/changelogs/component-card.yml Outdated Show resolved Hide resolved
Co-authored-by: Amy Leadem <93996430+amyleadem@users.noreply.github.com>
@mahoneycm
Copy link
Contributor Author

@amyleadem I agree about the alphabetization. I started adding it to the table with that in mind, but noticed the exdent values were put together and the other values were not alphabetized.

If we think it's a good idea to alphebetize, I'm happy to do so! Maybe we could come up with an organization pattern like organized by variant then alphabetized to get the best of both worlds?

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.

For now, what do you think about alphabetizing just the exdent variants items so that it goes like: body--exdent, footer-exdent, etc?

It might be worth discussing opening an issue to alphabetize variants across components since it doesn't seem very consistent.

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.

Looks good to me!

@mahoneycm mahoneycm merged commit 589d396 into main Sep 20, 2023
1 check passed
@mejiaj mejiaj deleted the cm-card-header-exdent-guidance branch February 21, 2024 15:46
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-Site - Card: Missing header--exdent from variant table.
3 participants