[WNMGDS-2838] Update all themes to make inline tool tips black

jack-ryan-nava-pbc commented 1 month ago

Summary

Ticket Changed the value pointed to in the tokens for '--tooltip-trigger__color` class.

How to test

Pull down the branch locally and build
Confirm that for each theme the color of the tooltip underline and text matches the color of the surrounding text.

Verify Screenshots look ok

Checklist

[X] Prefixed the PR title with the Jira ticket number as [WNMGDS-####] Title or [NO-TICKET] if this is unticketed work.
[x] Selected appropriate Type (only one) label for this PR, if it is a breaking change, label should only be Type: Breaking
[x] Selected appropriate Impacts, multiple can be selected.
[x] Selected appropriate release milestone

If this is a change to design:

[x] If visual regression image references have been changed, design MUST be assigned to review. In this instance, designer approval is a requirement before the PR can be merged.

If this is a change to code:

[x] Created or updated unit tests to cover any new or modified code
[x] If necessary, updated unit-test snapshots (yarn test:unit:update) and browser-test snapshots (yarn test:browser:update)

jack-ryan-nava-pbc commented 1 month ago

I don't know how possible this is with Figma tokens, but should this value actually be currentColor so the tooltip text/underline matches its surrounding text color?

I don't know if the goal is to make the tooltip match its surrounding text or to explicitly make it black for all scenarios (including the inverse background - if that's used at all, I don't know)

@zarahzachz I think this is a good callout. Looks like all of the Design Systems are using color--base in the text inherited from the <body>. I should be able to reference that variable in the themes via theme.color.base and give that a shot.

zarahzachz commented 4 weeks ago

I don't know how possible this is with Figma tokens, but should this value actually be currentColor so the tooltip text/underline matches its surrounding text color? I don't know if the goal is to make the tooltip match its surrounding text or to explicitly make it black for all scenarios (including the inverse background - if that's used at all, I don't know)

@zarahzachz I think this is a good callout. Looks like all of the Design Systems are using color--base in the text inherited from the <body>. I should be able to reference that variable in the themes via theme.color.base and give that a shot.

Oh, my recommendation wasn't to change it to color--base - it was to change it to currentColor, which I don't know is possible with the current token setup (Figma only supports a subset of CSS properties).

currentColor takes the color of whatever its parent has defined, meaning if this text were changed using a utility class like ds-u-color--error, the entire string (including the tooltip) would render red. I've attached a video explaining what I mean:

https://github.com/user-attachments/assets/231545e0-b7cd-4fef-9bd8-baf13fc82689

I don't think Figma supports currentColor, which may mean removing this token from the project and hardcoding it to currentColor (we do this on other components like SvgIcon fill). Since this token is only used in this one instance, I think an argument could be made in favor of deprecating the token. That would make this a breaking change, but since this is meant to go out in v12 I don't think that's an issue. Just things to talk about with your team.

jack-ryan-nava-pbc commented 3 weeks ago

Noting from discussion as of today:

We are continuing with these changes
We will investigate removing the tokens associated with Tooltips as part of our future Tooltip discovery work. That work will allow us to use currentColor

CMSgov / design-system