Skip to content

Conversation

@stefaniehansen
Copy link
Member

@stefaniehansen stefaniehansen commented Nov 19, 2020

Pull request checklist

Description of changes

Currently, we are attempting to capture keydown events on the tooltip (particularly the Escape and Ctrl key) even when the tooltip is closed. This means that in the event that the tooltip belongs to an element in a dialog or another component that listens to the same keydown event, the event is swallowed. We should check to see that the tooltip is open before attempting to handle this event (so that we don't stop event propagation).

Focus areas to test

(optional)

@msft-github-bot msft-github-bot added Component: Dialog needs cherry-pick Temporary label for PRs which may need to be cherry-picked to master labels Nov 19, 2020
@stefaniehansen stefaniehansen changed the title Fix/tooltip propagation Only attempt to handle keydown even for tooltip close if the tooltip is currently open. Nov 19, 2020
@stefaniehansen stefaniehansen changed the title Only attempt to handle keydown even for tooltip close if the tooltip is currently open. Fix tooltip swallowing keydown events for close in the event that the tooltip is already closed. Nov 19, 2020
@codesandbox-ci
Copy link

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 4cdc3b8:

Sandbox Source
Fluent UI Button Configuration
codesandbox-react-template Configuration

@msft-github-bot
Copy link
Contributor

msft-github-bot commented Nov 19, 2020

Perf Analysis

No significant results to display.

All results

Scenario Render type 7.0 Ticks PR Ticks Iterations Status
BaseButton mount 919 889 5000
Breadcrumb mount 42836 42866 5000
Checkbox mount 1544 1528 5000
CheckboxBase mount 1295 1267 5000
ChoiceGroup mount 4913 4896 5000
ComboBox mount 935 903 1000
CommandBar mount 7691 7739 1000
ContextualMenu mount 14564 14387 1000
DefaultButton mount 1156 1102 5000
DetailsRow mount 3557 3574 5000
DetailsRowFast mount 3603 3624 5000
DetailsRowNoStyles mount 3376 3440 5000
Dialog mount 1505 1488 1000
DocumentCardTitle mount 1826 1827 1000
Dropdown mount 2550 2529 5000
FocusTrapZone mount 1765 1747 5000
FocusZone mount 1818 1887 5000
IconButton mount 1743 1762 5000
Label mount 334 332 5000
Layer mount 1954 1969 5000
Link mount 437 447 5000
MenuButton mount 1456 1492 5000
MessageBar mount 2054 2065 5000
Nav mount 3224 3266 1000
OverflowSet mount 1432 1431 5000
Panel mount 1452 1469 1000
Persona mount 844 807 1000
Pivot mount 1419 1403 1000
PrimaryButton mount 1292 1283 5000
Rating mount 7539 7550 5000
SearchBox mount 1257 1278 5000
Shimmer mount 2527 2552 5000
Slider mount 1491 1479 5000
SpinButton mount 4985 5032 5000
Spinner mount 428 418 5000
SplitButton mount 3147 3185 5000
Stack mount 488 496 5000
StackWithIntrinsicChildren mount 1563 1576 5000
StackWithTextChildren mount 4626 4660 5000
SwatchColorPicker mount 10273 10203 5000
TagPicker mount 2673 2712 5000
TeachingBubble mount 51618 51738 5000
Text mount 427 412 5000
TextField mount 1343 1357 5000
Toggle mount 831 831 5000
button mount 109 106 5000

@size-auditor
Copy link

size-auditor bot commented Nov 19, 2020

Asset size changes

Project Bundle Baseline Size New Size Difference
office-ui-fabric-react office-ui-fabric-react-FloatingPicker 232.703 kB 232.729 kB ExceedsBaseline     26 bytes
office-ui-fabric-react office-ui-fabric-react-Tooltip 83.985 kB 84.011 kB ExceedsBaseline     26 bytes
office-ui-fabric-react office-ui-fabric-react-Breadcrumb 192.319 kB 192.345 kB ExceedsBaseline     26 bytes
office-ui-fabric-react office-ui-fabric-react-Pickers 275.903 kB 275.929 kB ExceedsBaseline     26 bytes
office-ui-fabric-react office-ui-fabric-react-SelectedItemsList 222.152 kB 222.178 kB ExceedsBaseline     26 bytes
office-ui-fabric-react office-ui-fabric-react-CommandBar 194.66 kB 194.686 kB ExceedsBaseline     26 bytes
office-ui-fabric-react office-ui-fabric-react-Facepile 202.999 kB 203.025 kB ExceedsBaseline     26 bytes
office-ui-fabric-react office-ui-fabric-react-PersonaCoin 111.817 kB 111.843 kB ExceedsBaseline     26 bytes
office-ui-fabric-react office-ui-fabric-react-Persona 111.817 kB 111.843 kB ExceedsBaseline     26 bytes

ExceedsTolerance Over Tolerance (1024 B) ExceedsBaseline Over Baseline BelowBaseline Below Baseline New New Deleted  Removed 1 kB = 1000 B

Baseline commit: 9812fcdcad2bc081ec41c4bee23a5e935533b3ea (build)

@behowell behowell merged commit cb2e970 into microsoft:7.0 Nov 19, 2020
@behowell behowell removed the needs cherry-pick Temporary label for PRs which may need to be cherry-picked to master label Nov 19, 2020
@msft-github-bot
Copy link
Contributor

🎉office-ui-fabric-react@v7.152.1 has been released which incorporates this pull request.:tada:

Handy links:

msft-github-bot pushed a commit that referenced this pull request Nov 25, 2020
#### Pull request checklist

- [ ] Addresses an existing issue: Fixes #0000
- [X] Include a change request file using `$ yarn change`

#### Description of changes

Currently, we are attempting to capture keydown events on the tooltip (particularly the Escape and Ctrl key) even when the tooltip is closed. This means that in the event that the tooltip belongs to an element in a dialog or another component that listens to the same keydown event, the event is swallowed. We should check to see that the tooltip is open before attempting to handle this event (so that we don't stop event propagation).

This is a cherry-pick from a change that was recently merged into v7.0 in this pull request.
#15990

#### Focus areas to test

(optional)
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.

4 participants