-
-
Notifications
You must be signed in to change notification settings - Fork 32.3k
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] Rework the implementation #12085
Conversation
8fbd01d
to
9ccb79d
Compare
@@ -27,13 +27,13 @@ module.exports = [ | |||
name: 'The size of all the modules of material-ui.', | |||
webpack: true, | |||
path: 'packages/material-ui/build/index.js', | |||
limit: '95.3 KB', | |||
limit: '94.3 KB', |
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.
🚀
9ccb79d
to
4cc7cfa
Compare
4cc7cfa
to
8bfb2df
Compare
@oliviertassinari, regarding #10909. Tooltips are used for Accessibility only when mounted ( |
@Z-AX If people are providing a string title property, we apply it to the children when the tooltip is closed. This is the nominal use case. We are good. |
@oliviertassinari Awesome job! |
Thanks! We'll check this out within the next sprint. |
Rad, this looks like exactly what my team needs for some perf issues we've been trying to get to the bottom of. I'm not familiar with your release process -- any idea roughly how long it might be until this patch is rolled into a release and available on NPM? I want to be sure to update when it's ready! Thanks for your work on this! 👍 |
I need it too for the office. We have a critical screen that surfers from an important performance issue. |
I'm sorry, I should have done smaller pull requests fixing one issue after another, but it was so much fun! Yes, it's closing 20 issues. Hopefully, it's not introducing any breaking changes.
Closes [ToolTip] Tables with ToolTips create extra space on screen #11350, Closes [Tooltip] Wrongly created initial tooltip positioning after resizing the window enough which results in scrollbars #9362
ClickAwayListener
with multiple children. It should be a single node. It's aReactDOM.findDOMNode
API limitation.disablePortal
property to the Portal.Closes [ClickAwayListener] Triggers on Select (and more) #12034
TransitionComponent
property.Closes Tooltip performances makes them unusable on large scales since v1.0.0-beta.40 #11431
Closes [Tooltip] Unmount Tooltip on exit and mount on enter #10909
PopperProps
property so people can provide their owncontainer
property.Closes How can I make elements such as select menus, tooltip, etc. render in a specific DOM element? #11454
Popper
component. I have been trying to upgrade react-popper to v1 in [Tooltip] Update react-popper #11862, but I had to revert with [Tooltip] Revert update react-popper #11920. I fail to see the value react-popper is providing over using Popper.js directly, worse, I find it harming performances (higher payload + many rerender) and code comprehension. So, I think that we are better off without. We are going down one level of abstraction. Save 1 kB gzipped 🚀The Popper is a wrapper on top of Popper.js with some additional feature (transition, portal and rtl). In the future, we could look into making Popper a wrapper on top of Modal so we can have some extra focus and keyboard feature.
Closes Cannot find module 'react-popper' #11197, Closes Support scroll and reposition of Popover #11796
Closes [Document] Bug of the
Menus
document's example #11882Closes Allow tooltips to function on disabled elements #11601
Closes [Tooltip] Tooltips should not disappear when hovered #11705.
Closes [Tooltip] How to customize the tooltip? #11467
Closes Autocomplete dropdown options hided by dialog. #11824, Closes Ability to override Modal container inside Popover #11733
Closes Tooltip stays displayed on top of MenuItem components of a Select component #11186
Closes Text in Tooltip component is blurry on secondary Retina Screen #12056.
modifiers.preventOverflow.boundariesElement = viewport
option of Popper.js.Closes [Tooltip] Flickers when overlaped with element it is attached to #10735, Closes Allow tooltips on tabs #10714, Closes Tooltip does not show when hover on MenuItem #10502
disablePortal
property. In the future, I would like to explore if we can make Popper rely on the Modal component. This will require to dive into the internal of the Modal. Like blueprint is doing with their Popover and Overlay component.Closes Use react-overlays directly? #10479
The implementations I have been benchmarking before / during the effort: