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

Formation: Update color-gray-dark to color-base #883

Merged
merged 4 commits into from
Nov 14, 2022

Conversation

jamigibbs
Copy link
Contributor

@jamigibbs jamigibbs commented Nov 2, 2022

Description

This update makes the base text in Formation to be $color-base instead of $color-gray-dark.

Closes department-of-veterans-affairs/vets-design-system-documentation#935

Acceptance criteria

  • The base color for body and #main uses $color-base instead of $color-gray-dark.

Definition of done

  • Changes have been tested in vets-website
  • Changes have been tested in IE11, if applicable
  • Documentation has been updated, if applicable
  • A link has been provided to the originating GitHub issue (or connected to it via ZenHub)
  • No sensitive information (i.e. PII/credentials/internal URLs/etc.) is captured in logging, hardcoded, or specs

@jamigibbs jamigibbs marked this pull request as ready for review November 2, 2022 17:14
@jamigibbs jamigibbs requested a review from a team as a code owner November 2, 2022 17:14
Copy link
Contributor

@bkjohnson bkjohnson left a comment

Choose a reason for hiding this comment

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

I see color-gray-dark used in other places where color-base might be more appropriate. For example:

.last-updated {
margin-top: 1.5em;
border-top: 2px solid $color-gray-light;
padding: 1em 0;
p {
font-size: 1em !important;
color: $color-gray-dark !important;

Can we see if some of the other uses of the color are using it because they wanted to match the base color, or because the specific gray is desired regardless of what the base is?

@jamigibbs
Copy link
Contributor Author

@mixin button-link

https://github.com/department-of-veterans-affairs/veteran-facing-services-tools/blob/master/packages/formation/sass/base/_b-mixins.scss#L159-L162

hr

https://github.com/department-of-veterans-affairs/veteran-facing-services-tools/blob/master/packages/formation/sass/base/_va.scss#L190-L194

.va-list--feature li

https://github.com/department-of-veterans-affairs/veteran-facing-services-tools/blob/master/packages/formation/sass/base/_va.scss#L335-L341

.card .number

https://github.com/department-of-veterans-affairs/veteran-facing-services-tools/blob/master/packages/formation/sass/base/_va.scss#L877-L887

.va-h-ruled--stars::after

https://github.com/department-of-veterans-affairs/veteran-facing-services-tools/blob/master/packages/formation/sass/base/_va.scss#L992-L1009

.va-note

https://github.com/department-of-veterans-affairs/veteran-facing-services-tools/blob/master/packages/formation/sass/base/_va.scss#L1100-L1104

.last-updated

https://github.com/department-of-veterans-affairs/veteran-facing-services-tools/blob/master/packages/formation/sass/base/_va.scss#L1317-L1320

.fa-angle-down

https://github.com/department-of-veterans-affairs/veteran-facing-services-tools/blob/master/packages/formation/sass/modules/_m-additional-info.scss#L22-L28

.va-nav-linkslist-heading

https://github.com/department-of-veterans-affairs/veteran-facing-services-tools/blob/master/packages/formation/sass/modules/_m-nav-linkslist.scss#L21-L28

.va-sidebarnav li > div

https://github.com/department-of-veterans-affairs/veteran-facing-services-tools/blob/master/packages/formation/sass/modules/_m-nav-sidebar.scss#L98

.process-step

https://github.com/department-of-veterans-affairs/veteran-facing-services-tools/blob/master/packages/formation/sass/modules/_m-process-list.scss#L49-L53

.va-table-list tr:nth-child(n+2)
https://github.com/department-of-veterans-affairs/veteran-facing-services-tools/blob/master/packages/formation/sass/modules/_m-table.scss#L82-L88

cc @humancompanion-usds

@humancompanion-usds
Copy link
Contributor

Leave alone

  • Button-link which is the disabled state.
  • hr
  • .va-list--feature li
  • .card .number
  • .va-h-ruled--stars::after
  • .last-updated
  • .fa-angle-down
  • .va-nav-linkslist-heading
  • .va-sidebarnav li > div
  • .process-step
  • .va-table-list tr:nth-child(n+2)

To be clear, I believe all of these are border colors. Only text color is changing and would need to stay in sync.

Needs more investigation

  • .va-note - Where is this in use, if anywhere? This is text color and thus may need to change. I'd need to know where this is used and if it's potentially a component or paragraph style or...?

@jamigibbs
Copy link
Contributor Author

.va-note - Where is this in use, if anywhere? This is text color and thus may need to change. I'd need to know where this is used and if it's potentially a component or paragraph style or...?

I did a search of the vets-website code for va-note and it did not return any results.

@humancompanion-usds
Copy link
Contributor

If @bkjohnson concurs I would recommend removing .va-note then. I believe we can move forward with this change.

Copy link
Contributor

@bkjohnson bkjohnson left a comment

Choose a reason for hiding this comment

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

.va-note looks good to remove - approving pre-emptively.

@@ -1,6 +1,6 @@
{
"name": "@department-of-veterans-affairs/formation",
"version": "7.0.2",
"version": "7.0.3",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bkjohnson I'm assuming this needs a version bump, correct?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, that's correct. Here are the instructions for how to update & publish. I believe step 4 "Create a release" doesn't work, but you can still manually create a github release.

@jamigibbs jamigibbs merged commit 214b7f4 into master Nov 14, 2022
@jamigibbs jamigibbs deleted the 935-update-base-color-formation branch November 14, 2022 15:02
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.

Correct base text color to be $color-base and not $color-gray-dark
3 participants