-
Notifications
You must be signed in to change notification settings - Fork 5
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
Footer update #96
Footer update #96
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few questions for now
tokens/components/footer/base.json
Outdated
@@ -122,7 +122,7 @@ | |||
}, | |||
"hover": { | |||
"text": { | |||
"value": "{link.hover.value}", | |||
"value": "{color.blue.900.value}", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason you switched from the global link-hover token? They both seem to use blue.900
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
woops, looks like I should have left it as link.hover
@@ -169,7 +169,7 @@ | |||
} | |||
}, | |||
"text": { | |||
"value": "{link.default.value}", | |||
"value": "{color.blue.800.value}", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason we switched from the link.default token?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this one, however, needs to change to 800 instead of 700. with the gray bg it doesn't meet AAA contrast standards and bumping it up to 800 fixes the contrast ratio.
Should I make a new token for this one or is it fine as is?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's fine as is
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This PR consists of the following work:
Updated some footer tokens for this PR