Skip to content
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.

Sign up for GitHub

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

Jump to bottom

SearchBox: Making role on root div opt in #15621

Merged
merged 2 commits into from
Oct 20, 2020
Merged

SearchBox: Making role on root div opt in #15621

merged 2 commits into from
Oct 20, 2020

Conversation

khmakoto
Copy link
Member

@khmakoto khmakoto commented Oct 20, 2020 •
edited
Loading

Pull request checklist

Description of changes

Cherry-pick of #15450.

Original PR description:

Before:

This JSX:

<SearchBox />

Renders this HTML:

<div role="search">
  <input ... />
</div>

After:

This JSX:

<SearchBox />
<SearchBox role="search" />

Renders this HTML:

<div>
  <input ... />
</div>

<div role="search">
  <input ... />
</div>

@khmakoto khmakoto mentioned this pull request Oct 20, 2020
Closed
40 tasks
@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 e1022ce:

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

@ecraig12345
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@msft-github-bot
Copy link
Contributor

Perf Analysis

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 863 828 5000
BaseButtonCompat mount 930 924 5000
Breadcrumb mount 163083 164219 5000
Checkbox mount 1487 1510 5000
CheckboxBase mount 1203 1259 5000
ChoiceGroup mount 4714 4725 5000
ComboBox mount 994 993 1000
CommandBar mount 22228 22179 1000
ContextualMenu mount 6225 6200 1000
DefaultButtonCompat mount 1127 1137 5000
DetailsRow mount 3656 3660 5000
DetailsRowFast mount 3491 3699 5000
DetailsRowNoStyles mount 3435 3414 5000
Dialog mount 1520 1488 1000
DocumentCardTitle mount 1863 1830 1000
Dropdown mount 3340 3422 5000
FocusTrapZone mount 1821 1815 5000
FocusZone mount 1841 1792 5000
IconButtonCompat mount 1716 1723 5000
Label mount 332 349 5000
Layer mount 1814 1821 5000
Link mount 469 485 5000
MenuButtonCompat mount 1458 1460 5000
MessageBar mount 2079 2129 5000
Nav mount 3273 3262 1000
OverflowSet mount 1060 1069 5000
Panel mount 1397 1473 1000
Persona mount 876 866 1000
Pivot mount 1374 1413 1000
PrimaryButtonCompat mount 1302 1276 5000
Rating mount 7411 7473 5000
SearchBox mount 1312 1331 5000
Shimmer mount 2479 2570 5000
Slider mount 1888 1902 5000
SpinButton mount 4943 5015 5000
Spinner mount 429 427 5000
SplitButtonCompat mount 3141 3165 5000
Stack mount 492 489 5000
StackWithIntrinsicChildren mount 1527 1504 5000
StackWithTextChildren mount 4532 4624 5000
SwatchColorPicker mount 10138 10180 5000
TagPicker mount 2745 2681 5000
TeachingBubble mount 11368 11466 5000
Text mount 418 404 5000
TextField mount 1330 1379 5000
ThemeProvider mount 1975 1965 5000
ThemeProvider virtual-rerender 641 631 5000
Toggle mount 817 798 5000
button mount 538 541 5000
buttonNative mount 114 107 5000

Perf Analysis (Fluent)

Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🎯 Avatar.Fluent 0.46 0.5 0.92:1 2000 913
🦄 Button.Fluent 0.11 0.22 0.5:1 5000 571
🔧 Checkbox.Fluent 0.66 0.34 1.94:1 1000 658
🎯 Dialog.Fluent 0.16 0.22 0.73:1 5000 799
🔧 Dropdown.Fluent 3.03 0.4 7.57:1 1000 3026
🔧 Icon.Fluent 0.14 0.06 2.33:1 5000 719
🦄 Image.Fluent 0.08 0.13 0.62:1 5000 424
🔧 Slider.Fluent 1.62 0.45 3.6:1 1000 1622
🔧 Text.Fluent 0.07 0.03 2.33:1 5000 371
🦄 Tooltip.Fluent 0.12 0.91 0.13:1 5000 576

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
AnimationMinimalPerf.default 450 410 1.1:1
AttachmentMinimalPerf.default 175 161 1.09:1
PortalMinimalPerf.default 184 170 1.08:1
RefMinimalPerf.default 259 244 1.06:1
SkeletonMinimalPerf.default 454 429 1.06:1
Image.Fluent 424 400 1.06:1
ButtonMinimalPerf.default 197 188 1.05:1
LabelMinimalPerf.default 456 434 1.05:1
ListMinimalPerf.default 517 494 1.05:1
SegmentMinimalPerf.default 394 377 1.05:1
AlertMinimalPerf.default 306 293 1.04:1
DropdownManyItemsPerf.default 768 741 1.04:1
MenuMinimalPerf.default 912 876 1.04:1
RadioGroupMinimalPerf.default 468 449 1.04:1
BoxMinimalPerf.default 378 367 1.03:1
CardMinimalPerf.default 573 555 1.03:1
CarouselMinimalPerf.default 475 459 1.03:1
ChatWithPopoverPerf.default 492 479 1.03:1
SliderMinimalPerf.default 1626 1584 1.03:1
SplitButtonMinimalPerf.default 3857 3728 1.03:1
TreeMinimalPerf.default 928 902 1.03:1
Avatar.Fluent 913 884 1.03:1
Checkbox.Fluent 658 638 1.03:1
Icon.Fluent 719 701 1.03:1
Slider.Fluent 1622 1568 1.03:1
Tooltip.Fluent 576 557 1.03:1
AvatarMinimalPerf.default 488 479 1.02:1
DialogMinimalPerf.default 813 795 1.02:1
FlexMinimalPerf.default 312 305 1.02:1
FormMinimalPerf.default 433 426 1.02:1
GridMinimalPerf.default 384 377 1.02:1
HeaderMinimalPerf.default 392 386 1.02:1
ListNestedPerf.default 592 582 1.02:1
StatusMinimalPerf.default 756 742 1.02:1
TableManyItemsPerf.default 2208 2156 1.02:1
TreeWith60ListItems.default 213 209 1.02:1
AttachmentSlotsPerf.default 1148 1133 1.01:1
ButtonUseCssPerf.default 861 853 1.01:1
HeaderSlotsPerf.default 813 805 1.01:1
ListWith60ListItems.default 958 945 1.01:1
MenuButtonMinimalPerf.default 1610 1590 1.01:1
PopupMinimalPerf.default 719 712 1.01:1
TextMinimalPerf.default 381 379 1.01:1
CustomToolbarPrototype.default 4017 3989 1.01:1
Dropdown.Fluent 3026 2998 1.01:1
ButtonSlotsPerf.default 616 615 1:1
ChatDuplicateMessagesPerf.default 432 434 1:1
EmbedMinimalPerf.default 1957 1954 1:1
InputMinimalPerf.default 1312 1313 1:1
LayoutMinimalPerf.default 443 441 1:1
ReactionMinimalPerf.default 418 416 1:1
IconMinimalPerf.default 693 694 1:1
TextAreaMinimalPerf.default 505 506 1:1
ToolbarMinimalPerf.default 972 968 1:1
TooltipMinimalPerf.default 829 826 1:1
AccordionMinimalPerf.default 152 154 0.99:1
DropdownMinimalPerf.default 2987 3012 0.99:1
ItemLayoutMinimalPerf.default 1299 1313 0.99:1
ProviderMergeThemesPerf.default 2103 2116 0.99:1
ProviderMinimalPerf.default 1020 1027 0.99:1
TableMinimalPerf.default 424 430 0.99:1
Dialog.Fluent 799 804 0.99:1
ButtonOverridesMissPerf.default 1701 1732 0.98:1
ImageMinimalPerf.default 381 390 0.98:1
ListCommonPerf.default 670 681 0.98:1
LoaderMinimalPerf.default 766 783 0.98:1
Button.Fluent 571 582 0.98:1
Text.Fluent 371 378 0.98:1
ButtonUseCssNestingPerf.default 1093 1124 0.97:1
ChatMinimalPerf.default 620 642 0.97:1
CheckboxMinimalPerf.default 2819 2917 0.97:1
DividerMinimalPerf.default 389 401 0.97:1
VideoMinimalPerf.default 647 672 0.96:1

@size-auditor
Copy link

size-auditor bot commented Oct 20, 2020

Asset size changes

⚠️ Insufficient baseline data to detect size changes

Unable to find bundle size details for Baseline commit: 820f5a3

Possible causes

  • The baseline build 820f5a3 is broken
  • The Size Auditor run for the baseline build 820f5a3 was not triggered

Recommendations

  • Please merge your branch for this Pull request with the latest master build and commit your changes once again

@khmakoto
Copy link
Member Author

@msft-github-bot merge in 1 minute

@msft-github-bot
Copy link
Contributor

Hello @khmakoto!

Because you've given me some instructions on how to help merge this pull request, I'll be modifying my merge approach. Here's how I understand your requirements for merging this pull request:

  • I won't merge this pull request until after the UTC date Tue, 20 Oct 2020 22:13:08 GMT, which is in 1 minute

If this doesn't seem right to you, you can tell me to cancel these instructions and use the auto-merge policy that has been configured for this repository. Try telling me "forget everything I just told you".

@msft-github-bot msft-github-bot merged commit b14a4ed into microsoft:master Oct 20, 2020
@khmakoto khmakoto deleted the searchboxRole branch October 20, 2020 22:14
SethDonohue pushed a commit to SethDonohue/fluentui that referenced this pull request Nov 2, 2020
@khmakoto
SearchBox: Making role on root div opt in (microsoft#15621)
43e7a08 
<!--
!!!!!!! IMPORTANT !!!!!!!

Due to work we're currently doing to prepare master branch for our version 8 beta release,
please hold-off submitting the PR until around October 12 if it's not urgent.
If it is urgent, please submit the PR targeting the 7.0 branch.

This change does not apply to react-northstar contributors.

See microsoft#15222 for more details. Sorry for the inconvenience and short notice.
-->

#### Pull request checklist

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

#### Description of changes

_Cherry-pick of microsoft#15450._

_Original PR description:_

## Before:

This JSX:
```
<SearchBox />
```
Renders this HTML: 
```jsx
<div role="search">
  <input ... />
</div>
```

## After:

This JSX:
```
<SearchBox />
<SearchBox role="search" />
```
Renders this HTML: 
```jsx
<div>
  <input ... />
</div>

<div role="search">
  <input ... />
</div>
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ecraig12345 ecraig12345 ecraig12345 approved these changes

@dzearing dzearing Awaiting requested review from dzearing

Assignees

@tomi-msft tomi-msft

Labels
AutoMerge Component: SearchBox
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@khmakoto @ecraig12345 @msft-github-bot @tomi-msft