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.

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

SearchBox: role=search is removed, but can be optionally defined via role prop. #15450

Merged
merged 7 commits into from
Oct 10, 2020
Merged

SearchBox: role=search is removed, but can be optionally defined via role prop. #15450

merged 7 commits into from
Oct 10, 2020

Conversation

dzearing
Copy link
Member

@dzearing dzearing commented Oct 9, 2020

Pull request checklist

Description of changes

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>

@ecraig12345
Copy link
Member

I think you have the wrong base branch.

@ecraig12345
Copy link
Member

I mean it looks like your local branch for this PR was based off of master. It should be based off of 7.0.

@msft-github-bot
Copy link
Contributor

msft-github-bot commented Oct 9, 2020

Perf Analysis

No significant results to display.

All results

Scenario Render type 7.0 Ticks PR Ticks Iterations Status
Avatar mount 910 935 5000
BaseButton mount 1013 1025 5000
Breadcrumb mount 43735 44145 5000
ButtonNext mount 599 625 5000
Checkbox mount 1756 1734 5000
CheckboxBase mount 1450 1460 5000
ChoiceGroup mount 5570 5565 5000
ComboBox mount 1016 1018 1000
CommandBar mount 8132 8130 1000
ContextualMenu mount 13880 14249 1000
DefaultButton mount 1283 1271 5000
DetailsRow mount 4026 4093 5000
DetailsRowFast mount 3992 4008 5000
DetailsRowNoStyles mount 3806 3810 5000
Dialog mount 1643 1660 1000
DocumentCardTitle mount 1928 1946 1000
Dropdown mount 2874 2832 5000
FocusTrapZone mount 1849 1861 5000
FocusZone mount 1912 2006 5000
IconButton mount 1995 1948 5000
Label mount 373 372 5000
Layer mount 2177 2171 5000
Link mount 506 482 5000
MenuButton mount 1627 1640 5000
MessageBar mount 2199 2174 5000
Nav mount 3579 3592 1000
OverflowSet mount 1522 1575 5000
Panel mount 1622 1614 1000
Persona mount 900 915 1000
Pivot mount 1555 1586 1000
PrimaryButton mount 1423 1439 5000
Rating mount 8529 8493 5000
SearchBox mount 1462 1459 5000
Shimmer mount 2910 2938 5000
Slider mount 1672 1683 5000
SpinButton mount 5621 5617 5000
Spinner mount 458 448 5000
SplitButton mount 3520 3566 5000
Stack mount 562 586 5000
StackWithIntrinsicChildren mount 2098 2129 5000
StackWithTextChildren mount 5632 5566 5000
SwatchColorPicker mount 11193 11435 5000
TagPicker mount 3027 2985 5000
TeachingBubble mount 53265 53322 5000
Text mount 468 471 5000
TextField mount 1645 1594 5000
ThemeProvider mount 1802 1797 5000
ThemeProvider virtual-rerender 664 689 5000
Toggle mount 908 928 5000
button mount 126 122 5000

Perf Analysis (Fluent)

Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🎯 Avatar.Fluent 0.51 0.53 0.96:1 2000 1021
🦄 Button.Fluent 0.13 0.22 0.59:1 5000 659
🔧 Checkbox.Fluent 0.71 0.39 1.82:1 1000 708
🦄 Dialog.Fluent 0.18 0.26 0.69:1 5000 915
🔧 Dropdown.Fluent 3.23 0.51 6.33:1 1000 3226
🔧 Icon.Fluent 0.17 0.07 2.43:1 5000 863
🦄 Image.Fluent 0.09 0.13 0.69:1 5000 464
🔧 Slider.Fluent 1.71 0.4 4.27:1 1000 1714
🔧 Text.Fluent 0.09 0.04 2.25:1 5000 434
🦄 Tooltip.Fluent 0.13 16.75 0.01:1 5000 633

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
AccordionMinimalPerf.default 174 0 Infinity:1
AttachmentMinimalPerf.default 199 0 Infinity:1
AvatarMinimalPerf.default 569 0 Infinity:1
BoxMinimalPerf.default 432 0 Infinity:1
ButtonMinimalPerf.default 224 0 Infinity:1
ButtonSlotsPerf.default 719 0 Infinity:1
ButtonUseCssPerf.default 934 0 Infinity:1
ButtonUseCssNestingPerf.default 1226 0 Infinity:1
CardMinimalPerf.default 719 0 Infinity:1
CarouselMinimalPerf.default 512 0 Infinity:1
ChatDuplicateMessagesPerf.default 467 0 Infinity:1
ChatWithPopoverPerf.default 529 0 Infinity:1
CheckboxMinimalPerf.default 3160 0 Infinity:1
DialogMinimalPerf.default 902 0 Infinity:1
DividerMinimalPerf.default 426 0 Infinity:1
DropdownManyItemsPerf.default 876 0 Infinity:1
DropdownMinimalPerf.default 3152 0 Infinity:1
EmbedMinimalPerf.default 2211 0 Infinity:1
FlexMinimalPerf.default 344 0 Infinity:1
FormMinimalPerf.default 515 0 Infinity:1
GridMinimalPerf.default 424 0 Infinity:1
HeaderMinimalPerf.default 457 0 Infinity:1
HeaderSlotsPerf.default 913 0 Infinity:1
ImageMinimalPerf.default 466 0 Infinity:1
ItemLayoutMinimalPerf.default 1531 0 Infinity:1
LabelMinimalPerf.default 502 0 Infinity:1
ListCommonPerf.default 766 0 Infinity:1
ListMinimalPerf.default 572 0 Infinity:1
ListNestedPerf.default 692 0 Infinity:1
LoaderMinimalPerf.default 816 0 Infinity:1
MenuMinimalPerf.default 1010 0 Infinity:1
MenuButtonMinimalPerf.default 1805 0 Infinity:1
PopupMinimalPerf.default 799 0 Infinity:1
PortalMinimalPerf.default 185 0 Infinity:1
ProviderMergeThemesPerf.default 2200 0 Infinity:1
RadioGroupMinimalPerf.default 530 0 Infinity:1
ReactionMinimalPerf.default 496 0 Infinity:1
RefMinimalPerf.default 272 0 Infinity:1
SegmentMinimalPerf.default 425 0 Infinity:1
SkeletonMinimalPerf.default 546 0 Infinity:1
StatusMinimalPerf.default 856 0 Infinity:1
IconMinimalPerf.default 796 0 Infinity:1
TableManyItemsPerf.default 2534 0 Infinity:1
TableMinimalPerf.default 492 0 Infinity:1
TextMinimalPerf.default 432 0 Infinity:1
TextAreaMinimalPerf.default 594 0 Infinity:1
CustomToolbarPrototype.default 4208 0 Infinity:1
ToolbarMinimalPerf.default 1080 0 Infinity:1
TooltipMinimalPerf.default 928 0 Infinity:1
TreeMinimalPerf.default 1016 0 Infinity:1
VideoMinimalPerf.default 753 0 Infinity:1
Avatar.Fluent 1021 0 Infinity:1
Button.Fluent 659 0 Infinity:1
Checkbox.Fluent 708 0 Infinity:1
Icon.Fluent 863 0 Infinity:1
Text.Fluent 434 0 Infinity:1
SplitButtonMinimalPerf.default 4246 1 4246:1
Dropdown.Fluent 3226 1 3226:1
ButtonOverridesMissPerf.default 1874 1 1874:1
SliderMinimalPerf.default 1716 1 1716:1
Slider.Fluent 1714 1 1714:1
InputMinimalPerf.default 1435 1 1435:1
AttachmentSlotsPerf.default 1274 1 1274:1
ProviderMinimalPerf.default 1143 1 1143:1
ListWith60ListItems.default 1056 1 1056:1
Dialog.Fluent 915 1 915:1
ChatMinimalPerf.default 750 1 750:1
Tooltip.Fluent 633 1 633:1
AnimationMinimalPerf.default 496 1 496:1
LayoutMinimalPerf.default 493 1 493:1
Image.Fluent 464 1 464:1
AlertMinimalPerf.default 364 1 364:1
TreeWith60ListItems.default 232 1 232:1

@size-auditor
Copy link

size-auditor bot commented Oct 9, 2020

Asset size changes

Project Bundle Baseline Size New Size Difference
office-ui-fabric-react office-ui-fabric-react-SearchBox 180.018 kB 180.027 kB ExceedsBaseline     9 bytes

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

Baseline commit: 40edbe0aa78c0d1a56a3609318431fb4ceb4abe5 (build)

Copy link
Member

@khmakoto khmakoto left a comment

Choose a reason for hiding this comment

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

Seems like you are missing snapshot updates in @fluentui/react-examples.

@dzearing dzearing merged commit 23ec52b into microsoft:7.0 Oct 10, 2020
@msft-github-bot
Copy link
Contributor

🎉[email protected] has been released which incorporates this pull request.:tada:

Handy links:

@msft-github-bot
Copy link
Contributor

🎉@fluentui/[email protected] has been released which incorporates this pull request.:tada:

Handy links:

msft-github-bot pushed a commit that referenced this pull request Oct 20, 2020
<!--
!!!!!!! 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 #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 #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>
```
SethDonohue pushed a commit to SethDonohue/fluentui that referenced this pull request Nov 2, 2020
<!--
!!!!!!! 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SearchBox role is hardcoded and can not be changed.
6 participants