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

Convert tooltips and popovers to CSS vars #34622

Merged
merged 6 commits into from
Mar 8, 2022
Merged

Convert tooltips and popovers to CSS vars #34622

merged 6 commits into from
Mar 8, 2022

Conversation

mdo
Copy link
Member

@mdo mdo commented Jul 29, 2021

Summary

This PR starts the process of migrating tooltips and popovers to use CSS variables. In doing so, I've deprecated three Sass variables in an effort to improve customization via CSS variables. See _variables.scss for the changes there.

Beyond that, this replaces the bulk of our Sass for tooltips and popovers with customizable CSS variables. Most are reassigned from the Sass variables and respect the same name, but as mentioned above, some deviation was required for a more sensible future friendly path.

Originally I expected tooltips and popovers to be a clusterfuck to customize, but thanks to the customClass option we recently added, this is basically a breeze. I've added the data attributes, wrote some basic styles, and added some lovely docs to explain and demonstrate the situation for folks.

Fixes #34221.

Preview

Todos

  • Document deprecations maybe in the Migration guide?
  • Confirm all CSS vars are properly being used (fusv doesn't do that for us)
  • Adjust bundlewatch

@mdo mdo force-pushed the css-vars-tooltips branch 3 times, most recently from 4225950 to 439718e Compare September 9, 2021 03:21
@mdo mdo changed the title Convert tooltips to CSS vars Convert tooltips and popovers to CSS vars Sep 9, 2021
@mdo mdo marked this pull request as ready for review September 9, 2021 03:25
@mdo mdo requested a review from a team as a code owner September 9, 2021 03:25
@mdo mdo force-pushed the css-vars-tooltips branch 5 times, most recently from b12a8e0 to 55b664e Compare October 5, 2021 21:40
@mdo mdo force-pushed the css-vars-tooltips branch from 55b664e to eb3ce56 Compare November 2, 2021 04:43
@mdo
Copy link
Member Author

mdo commented Nov 2, 2021

@ffoodd Would love your eyes on this, #34443, and #34443. In particular, I'm unsure how we should setup the font-size values while still supporting RFS. Rest of this is feeling good to me though!

@mdo mdo mentioned this pull request Nov 2, 2021
41 tasks
@ffoodd
Copy link
Member

ffoodd commented Dec 7, 2021

Regarding font-sizes and RFS, I don't see any other option if we want to use RFS.

Note: regarding my suggestion PR using fallback values, we might check that value is above $rfs-base-value: if so, define var()'s fallback value, else use rfs() as you currently do. Not sure the extra effort is needed though.

ffoodd
ffoodd previously requested changes Dec 7, 2021
@mdo mdo requested a review from ffoodd February 8, 2022 00:12
@mdo mdo force-pushed the css-vars-tooltips branch from cf5d1af to f9658e4 Compare February 16, 2022 16:52
@mdo
Copy link
Member Author

mdo commented Feb 16, 2022

Updated this PR to also split the padding values for tooltips, popover headers, and popover bodies.

@mdo mdo force-pushed the css-vars-tooltips branch 2 times, most recently from d96e109 to 78a37f8 Compare February 17, 2022 00:59
@mdo mdo force-pushed the css-vars-tooltips branch from 78a37f8 to e1acfed Compare February 25, 2022 17:07
Copy link
Member

@ffoodd ffoodd left a comment

Choose a reason for hiding this comment

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

A single question regarding browser support, but looks great already :)

@mdo mdo force-pushed the css-vars-tooltips branch from e1acfed to 8804395 Compare February 28, 2022 22:48
@mdo mdo requested a review from ffoodd February 28, 2022 22:48
@mdo mdo force-pushed the css-vars-tooltips branch 3 times, most recently from 7fe4833 to f7f7a35 Compare March 5, 2022 01:27
@mdo mdo force-pushed the css-vars-tooltips branch from 69421a1 to 0ba4c6a Compare March 8, 2022 22:40
@mdo mdo dismissed ffoodd’s stale review March 8, 2022 22:52

Out of date, changes addressed

@mdo mdo merged commit 3a57da4 into main Mar 8, 2022
@mdo mdo deleted the css-vars-tooltips branch March 8, 2022 22:53
@tagliala tagliala mentioned this pull request Sep 3, 2022
3 tasks
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.

v5: Change tooltip background & color to css variables for easier customization
3 participants