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

Switch to design system tootips in tccp #8698

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

wpears
Copy link
Member

@wpears wpears commented Dec 20, 2024

Depends on #8694

Now that tooltips are available in the design system, we should use them instead of our own custom implementation in tccp. This does that.

Things to note:

  • The design-system doesn't forward the tooltip sass, so it has to be included specifically. I think this may be an oversight that we should correct in the design system index.scss, but I'll defer to @anselmbradford
  • The data-tooltip needs to match the template id of the tooltip contents, so I put a little counter in the existing tooltip macro. This works more or less fine for how we render pages.

Testing

@anselmbradford
Copy link
Member

anselmbradford commented Dec 20, 2024

The design-system doesn't forward the tooltip sass, so it has to be included specifically. I think this may be an oversight that we should correct in the design system index.scss, but I'll defer to @anselmbradford

You're talking about the need to add @use '@cfpb/cfpb-design-system/src/components/cfpb-tooltips/tooltip' as *;, right?

We do have a precedent of going into the components directory for some things, e.g. https://github.com/cfpb/consumerfinance.gov/blob/main/cfgov/unprocessed/css/skip-nav.scss#L3, but I agree that doesn't seem great and have treated that as a discouraged pattern that we may want to refactor away from.

That said, for the tooltips, since their JS has a separate export maybe the Sass should work that way too?

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.

3 participants