-
-
Notifications
You must be signed in to change notification settings - Fork 29
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
[Tooltip] Component and Hook #264
Conversation
@atomiks Hilarious to see this in one day 🤣🔥
|
Yeah 😄 as the review comment above says, it's really 2 years of work, compressed into 1 day since I can just reuse all that work I did. |
|
The main use case where this is necessary for touch input is the "infotip" pattern, where an info icon can be tapped to reveal information. It's not a button tool that performs an action, as the action itself is to show the infotip. For keyboard usage, this pattern may require a Another potential solution is to use long press to show the tooltip on touch devices, rather than a tap. This lets them see the description without triggering the button. Probably somewhat rare? |
8d4d38b
to
1efbbb8
Compare
22e2843
to
11f1654
Compare
4966245
to
b3f0b25
Compare
They can use a global provider if they want (the rest of the app remains performant). A provider allow them to change the props for a certain group of tooltips by overriding a "global" one if necessary. Keeping it fully global goes against React's model for the most part.
We've completely re-thought about what tooltips should be. It does not work on mobile intentionally as tooltips are an anti-pattern on touch devices. The docs explain this further and what to use instead.
These are common issues we've thought of so far and we've either avoided them or know that they require the user to workaround it explicitly in their code |
Tooltip.Group
Delay
Docs Content
|
It's a bit confusing in
The thing with the |
docs/data/base/components/tooltip/UnstyledTooltipDelayGroup.tsx
Outdated
Show resolved
Hide resolved
@@ -4,7 +4,7 @@ const fse = require('fs-extra'); | |||
const errorCodesPath = path.resolve(__dirname, './public/static/error-codes.json'); | |||
|
|||
const { version: transformRuntimeVersion } = fse.readJSONSync( | |||
require.resolve('@babel/runtime-corejs2/package.json'), | |||
require.resolve('@babel/runtime-corejs3/package.json'), |
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 is already on master, isn't it?
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.
Yes, I added it here to validate it first and because the docs/demos were broken without it before the other PR got merged
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.
Looks good to me!
Closes mui/material-ui#32
Preview: https://deploy-preview-264--base-ui.netlify.app/base-ui/react-tooltip/
Transition experiment: https://deploy-preview-264--base-ui.netlify.app/experiments/tooltip/
TODO: