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

refactor: remove tokens redundant after CSS rewrite #230

Merged
merged 9 commits into from
Jul 10, 2024

Conversation

melaniebmn
Copy link
Collaborator

@melaniebmn melaniebmn commented Feb 21, 2024

Summary | Résumé

As part of the CSS rewrite some of the tokens have become redundant.

gcds-link was integrated into the following components:

  • gcds-breadcrumbs
  • gcds-card
  • gcds-footer
  • gcds-lang-toggle

gcds-text was integrated into the following components:

  • gcds-card
  • gcds-date-modified
  • gcds-error-message
  • gcds-hint

gcds-heading was integrated into the following components:

  • gcds-error-summary
  • gcds-stepper

Note: This PR shouldn't be merged until the CSS rewrite changes are published and the component tokens in Figma are updated. I will keep it in draft mode for now.

@adorayi
Copy link

adorayi commented Jul 8, 2024

Need a touchpoint with Design before merging this PR.

…eading, and gcds-text into existing components (#231)

* remove redundant tokens after integrating gcds-link into gcds-lang-toggle

* refactor: remove redundant tokens after integrating gcds-link into gcds-card

* refactor: remove redundant tokens after integrating gcds-link into gcds-breadcrumbs

* refactor: remove redundant tokens after integrating gcds-link into gcds-footer

* fix: revert removing footer-main-text token

* refactor: remove redundant tokens after integrating setting regular size for gcds-link component in existing components

* refactor: remove redundant tokens after integrating gcds-text and gcds-heading into existing components (#232)

* refactor: remove redundant tokens after integrating gcds-text into gcds-card

* refactor: remove redundant tokens after integrating gcds-text into gcds-card

* refactor: remove redundant tokens after integrating gcds-text into gcds-error-message

* refactor: remove redundant tokens after integrating gcds-text into gcds-hint

* refactor: remove redundant tokens after integrating gcds-text into gcds-date-modified

* chore: revert accidentally updated dates

* refactor: remove redundant tokens after integrating gcds-heading into gcds-stepper

* refactor: remove redundant tokens after integrating gcds-heading into gcds-error-summary
* refactor: keep red error colour for error summary links

* refactor: revert red error colour for error summary links
@melaniebmn melaniebmn force-pushed the update-lang-toggle-padding branch from 95d708a to e2857e1 Compare July 9, 2024 23:32
@melaniebmn melaniebmn marked this pull request as ready for review July 9, 2024 23:54
@melaniebmn melaniebmn requested review from ethanWallace and daine July 9, 2024 23:55
@melaniebmn
Copy link
Collaborator Author

@ethanWallace can you confirm that we won't be using the --gcds-card-hover-title-text-decoration-thickness token for the card 2.0 design anymore? I checked your draft PR and it looks like that is the only token that can be removed.

daine
daine previously approved these changes Jul 10, 2024
Copy link
Collaborator

@daine daine left a comment

Choose a reason for hiding this comment

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

LGTM, we can wait for ethan to confirm usage of that token

@ethanWallace
Copy link
Collaborator

@ethanWallace can you confirm that we won't be using the --gcds-card-hover-title-text-decoration-thickness token for the card 2.0 design anymore? I checked your draft PR and it looks like that is the only token that can be removed.

I can confirm that token is no longer being used.

@melaniebmn
Copy link
Collaborator Author

@ethanWallace @daine I removed the unused card token, PR should be good to go.

Copy link
Collaborator

@ethanWallace ethanWallace left a comment

Choose a reason for hiding this comment

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

LGTM!

@melaniebmn melaniebmn merged commit d819ecf into main Jul 10, 2024
1 check passed
@melaniebmn melaniebmn deleted the update-lang-toggle-padding branch July 10, 2024 17:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants