Skip to content
This repository has been archived by the owner on Mar 4, 2020. It is now read-only.

fix(Toolbar): ToolbarMenuItem doesn't focus menu trigger element after 'onClick' was executed #2367

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

kolaps33
Copy link
Collaborator

@kolaps33 kolaps33 commented Feb 17, 2020

Fixing the issue:
#2249

Fixes #2249

@kolaps33 kolaps33 changed the title fix(Toolbar): ToolbarMenuItem doesn't focus menu trigger element after onClick was executed fix(Toolbar): ToolbarMenuItem doesn't focus menu trigger element after 'onClick' was executed Feb 17, 2020
@layershifter
Copy link
Member

Can we add a E2E/Unit test for this?

@@ -9,6 +9,7 @@ const doList = [
`aria-labelledby` props). Refer to{' '}
{link('toolbar(role)', 'https://www.w3.org/WAI/PF/aria/roles#toolbar')} for details.
</Text>,
'If `Toolbar` contains menu, then focus will need be handled after `onClick` is executed on `menuItem`.'
Copy link
Contributor

Choose a reason for hiding this comment

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

'If Toolbar contains menu, the menu closes after clicking on one of the menu items. To prevent losing focus, move it manually in the onClick handler.

@DustyTheBot
Copy link
Collaborator


Fails
🚫 Invalid entry format in CHANGELOG.md: >+- Fix `Toolbar` - ToolbarMenuItem doesn't focus menu trigger element after 'onClick' was executed ([#2367](https://github.com//pull/2367)) <

The correct format is: - description @githubname ([#DDDD](https://github.com/microsoft/fluent-ui-react/pull/DDDD)

Perf comparison

Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🔧 Avatar.Fluent 0.48 0.37 1.3:1 2000 963
🦄 Button.Fluent 0.12 0.2 0.6:1 1000 120
🔧 Checkbox.Fluent 0.83 0.34 2.44:1 1000 828
🔧 Dialog.Fluent 0.32 0.16 2:1 5000 1577
🔧 Dropdown.Fluent 3.29 0.34 9.68:1 1000 3291
🔧 Icon.Fluent 0.12 0.03 4:1 5000 589
🦄 Image.Fluent 0.05 0.08 0.63:1 5000 230
🔧 Slider.Fluent 1.45 0.32 4.53:1 1000 1451
🔧 Text.Fluent 0.05 0.02 2.5:1 5000 254
🦄 Tooltip.Fluent 0.1 17.99 0.01:1 5000 483

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
VideoMinimalPerf.default 704 0 Infinity:1
SplitButtonMinimalPerf.default 12877 11310 1.14:1
Tooltip.Fluent 483 429 1.13:1
Button.Fluent 120 110 1.09:1
ButtonMinimalPerf.default 120 111 1.08:1
DropdownManyItemsPerf.default 439 406 1.08:1
FormMinimalPerf.default 742 689 1.08:1
TreeWith60ListItems.default 246 229 1.07:1
ProviderMinimalPerf.default 568 537 1.06:1
BoxMinimalPerf.default 231 221 1.05:1
FlexMinimalPerf.default 348 334 1.04:1
RefMinimalPerf.default 151 145 1.04:1
TableMinimalPerf.default 581 559 1.04:1
Slider.Fluent 1451 1397 1.04:1
AccordionMinimalPerf.default 216 209 1.03:1
AttachmentMinimalPerf.default 919 890 1.03:1
InputMinimalPerf.default 919 895 1.03:1
ListWith60ListItems.default 160 156 1.03:1
SegmentMinimalPerf.default 1668 1620 1.03:1
TextAreaMinimalPerf.default 3087 2983 1.03:1
TooltipMinimalPerf.default 640 621 1.03:1
Image.Fluent 230 223 1.03:1
AnimationMinimalPerf.default 464 455 1.02:1
ButtonSlotsPerf.default 580 569 1.02:1
ItemLayoutMinimalPerf.default 1651 1613 1.02:1
ListNestedPerf.default 672 660 1.02:1
ProviderMergeThemesPerf.default 1047 1025 1.02:1
CheckboxMinimalPerf.default 3618 3587 1.01:1
HeaderMinimalPerf.default 415 412 1.01:1
IconMinimalPerf.default 274 272 1.01:1
ImageMinimalPerf.default 227 224 1.01:1
LabelMinimalPerf.default 779 774 1.01:1
ListMinimalPerf.default 298 294 1.01:1
ChatMinimalPerf.default 1621 1619 1:1
DropdownMinimalPerf.default 3297 3306 1:1
EmbedMinimalPerf.default 6019 6014 1:1
HeaderSlotsPerf.default 1262 1267 1:1
HierarchicalTreeMinimalPerf.default 751 748 1:1
MenuMinimalPerf.default 1826 1823 1:1
PortalMinimalPerf.default 214 214 1:1
Avatar.Fluent 963 962 1:1
Checkbox.Fluent 828 827 1:1
AlertMinimalPerf.default 555 561 0.99:1
AttachmentSlotsPerf.default 3149 3168 0.99:1
GridMinimalPerf.default 759 769 0.99:1
LayoutMinimalPerf.default 481 487 0.99:1
MenuButtonMinimalPerf.default 1459 1481 0.99:1
PopupMinimalPerf.default 306 310 0.99:1
RadioGroupMinimalPerf.default 383 386 0.99:1
SliderMinimalPerf.default 1851 1862 0.99:1
StatusMinimalPerf.default 245 248 0.99:1
Dropdown.Fluent 3291 3328 0.99:1
CustomToolbarPrototype.default 3397 3464 0.98:1
Dialog.Fluent 1577 1605 0.98:1
Text.Fluent 254 259 0.98:1
CarouselMinimalPerf.default 1754 1815 0.97:1
ChatWithPopoverPerf.default 539 558 0.97:1
ListCommonPerf.default 709 731 0.97:1
ReactionMinimalPerf.default 2413 2500 0.97:1
TreeMinimalPerf.default 906 930 0.97:1
ChatDuplicateMessagesPerf.default 521 542 0.96:1
DividerMinimalPerf.default 857 892 0.96:1
AvatarMinimalPerf.default 499 527 0.95:1
ToolbarMinimalPerf.default 721 760 0.95:1
Icon.Fluent 589 620 0.95:1
DialogMinimalPerf.default 1573 1704 0.92:1
TextMinimalPerf.default 260 301 0.86:1
LoaderMinimalPerf.default 2027 2425 0.84:1

Generated by 🚫 dangerJS

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ToolbarMenuItem should not focus menu trigger after select.
5 participants