-
Notifications
You must be signed in to change notification settings - Fork 2.8k
DRAFT: Popover and Anchor Positioning based Tooltip #35261
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
base: master
Are you sure you want to change the base?
DRAFT: Popover and Anchor Positioning based Tooltip #35261
Conversation
📊 Bundle size reportUnchanged fixtures
|
Pull request demo site: URL |
packages/react-components/react-color-picker/library @microsoft/cxe-prg | ||
packages/react-components/react-color-picker/stories @microsoft/cxe-prg | ||
packages/react-components/component-selector-preview/library @microsoft/teams-prg | ||
packages/react-components/component-selector-preview/stories @microsoft/teams-prg |
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.
🕵🏾♀️ visual changes to review in the Visual Change Report
vr-tests-react-components/CalendarCompat 4 screenshots
Image Name | Diff(in Pixels) | Image Type |
---|---|---|
vr-tests-react-components/CalendarCompat.multiDayView - High Contrast.default.chromium.png | 1236 | Changed |
vr-tests-react-components/CalendarCompat.multiDayView.default.chromium_1.png | 403 | Changed |
vr-tests-react-components/CalendarCompat.multiDayView - RTL.default.chromium.png | 496 | Changed |
vr-tests-react-components/CalendarCompat.multiDayView - Dark Mode.default.chromium.png | 1111 | Changed |
vr-tests-react-components/Positioning 2 screenshots
Image Name | Diff(in Pixels) | Image Type |
---|---|---|
vr-tests-react-components/Positioning.Positioning end.chromium.png | 129 | Changed |
vr-tests-react-components/Positioning.Positioning end.updated 2 times.chromium.png | 29 | Changed |
vr-tests/Callout 3 screenshots
Image Name | Diff(in Pixels) | Image Type |
---|---|---|
vr-tests/Callout.Top right edge.default.chromium.png | 1134 | Changed |
vr-tests/Callout.Rendering callout attached to a rectangle.default.chromium.png | 1832 | Changed |
vr-tests/Callout.Beak 25.default.chromium.png | 2185 | Changed |
vr-tests/Coachmark 1 screenshots
Image Name | Diff(in Pixels) | Image Type |
---|---|---|
vr-tests/Coachmark.Collapsed.default.chromium.png | 159 | Changed |
vr-tests/Keytip 1 screenshots
Image Name | Diff(in Pixels) | Image Type |
---|---|---|
vr-tests/Keytip.Root.default.chromium.png | 55 | Changed |
vr-tests/react-charting-LineChart 2 screenshots
Image Name | Diff(in Pixels) | Image Type |
---|---|---|
vr-tests/react-charting-LineChart.Multiple - Dark Mode.default.chromium.png | 181 | Changed |
vr-tests/react-charting-LineChart.Multiple - RTL.default.chromium.png | 200 | Changed |
vr-tests/react-charting-VerticalBarChart 1 screenshots
Image Name | Diff(in Pixels) | Image Type |
---|---|---|
vr-tests/react-charting-VerticalBarChart.Basic - Secondary Y Axis.default.chromium.png | 3 | Changed |
There were 1 duplicate changes discarded. Check the build logs for more information.
...state.content.style, | ||
}; | ||
|
||
// When this tooltip is visible, hide any other tooltips, and register 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.
Do you think it's possible to enforce this with auto
? https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Global_attributes/popover#auto
In other words, could we eliminate all this code to hide other tooltips?
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.
Theoretically yes, though we really want hint
here. Using auto would dismiss other popovers, e.g. Menu which we don't want. hint
also has issues when we want to preserve the ignoreNextFocusEventRef
and the delay on dismissing after mousing out of the trigger. If we want that to remain unchanged I think we'll need to stick with manual
and show/hide it with the existing logic.
color: tokens.colorNeutralForegroundStaticInverted, | ||
}, | ||
|
||
arrow: createArrowStyles({ arrowHeight }), |
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.
Have you been able to get this working? I struggled with adding it and maintaining the existing DOM structure when prototyping this.
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.
Not yet, no. It's also complicated by the position fallbacks. Currently we don't know which fallback position was used in code so we can't manually move the arrow in response
Previous Behavior
New Behavior
Related Issue(s)