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

Add missing typography-color-secondary to Select label #362

Merged
merged 1 commit into from
May 28, 2024

Conversation

ty2k
Copy link
Contributor

@ty2k ty2k commented May 28, 2024

Following #354 , this adds a missing color to the Select component's label. Without this PR, the browser default black is being applied to the label:

Storybook from OpenShift dev environment

After this PR:
Storybook from local dev environment

This makes the "main" label have the same color as the "(optional)" tag when it is present. Note that we are currently missing a canonical example of what this label should actually look like with the "(optional)" tag in our pre-release Figma component file:

Figma Dropdown example from May 28

@ty2k ty2k added the bug Something isn't working label May 28, 2024
@ty2k ty2k requested a review from mkernohanbc May 28, 2024 23:02
@ty2k ty2k self-assigned this May 28, 2024
@mkernohanbc mkernohanbc requested a review from Philip-Cheung May 28, 2024 23:06
Copy link
Contributor

@mkernohanbc mkernohanbc left a comment

Choose a reason for hiding this comment

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

My interpretation is that the main label should use --typography-color-primary, and the "(optional)" label should use --typography-color-secondary, but I think that @Philip-Cheung has the decision on what's correct here — adding him as a reviewer.

(Philip, this relates to the Figma change requested in https://github.com/orgs/bcgov/projects/106/views/3?pane=issue&itemId=63792131)

@ty2k ty2k merged commit cdc88bf into main May 28, 2024
1 check passed
@ty2k ty2k deleted the bugfix/select-label-color branch May 28, 2024 23:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants