ColorPicker (v8): convert tests to use testing-library #22264
Conversation
|
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 84a7f97:
|
Asset size changesSize Auditor did not detect a change in bundle size for any component! Baseline commit: 69bcdf2f52fe6a8bf7d5f7ca71be74d753202356 (build) |
📊 Bundle size report🤖 This report was generated against 69bcdf2f52fe6a8bf7d5f7ca71be74d753202356 |
Perf Analysis (
|
| Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
|---|---|---|---|---|---|
| BaseButton | mount | 977 | 931 | 5000 | |
| Breadcrumb | mount | 2915 | 2822 | 1000 | |
| Checkbox | mount | 1592 | 1596 | 5000 | |
| CheckboxBase | mount | 1312 | 1374 | 5000 | |
| ChoiceGroup | mount | 4975 | 4977 | 5000 | |
| ComboBox | mount | 1017 | 1139 | 1000 | |
| CommandBar | mount | 11200 | 11012 | 1000 | |
| ContextualMenu | mount | 12731 | 12756 | 1000 | |
| DefaultButton | mount | 1229 | 1234 | 5000 | |
| DetailsRow | mount | 4106 | 4149 | 5000 | |
| DetailsRowFast | mount | 3948 | 3986 | 5000 | |
| DetailsRowNoStyles | mount | 3892 | 3807 | 5000 | |
| Dialog | mount | 2299 | 2311 | 1000 | |
| DocumentCardTitle | mount | 189 | 174 | 1000 | |
| Dropdown | mount | 3429 | 3470 | 5000 | |
| FocusTrapZone | mount | 1887 | 1969 | 5000 | |
| FocusZone | mount | 1837 | 1915 | 5000 | |
| IconButton | mount | 1808 | 1815 | 5000 | |
| Label | mount | 364 | 359 | 5000 | |
| Layer | mount | 3043 | 3100 | 5000 | |
| Link | mount | 467 | 475 | 5000 | |
| MenuButton | mount | 1607 | 1572 | 5000 | |
| MessageBar | mount | 2195 | 2277 | 5000 | |
| Nav | mount | 3395 | 3463 | 1000 | |
| OverflowSet | mount | 1114 | 1145 | 5000 | |
| Panel | mount | 2220 | 2223 | 1000 | |
| Persona | mount | 1064 | 1064 | 1000 | |
| Pivot | mount | 1490 | 1483 | 1000 | |
| PrimaryButton | mount | 1325 | 1373 | 5000 | |
| Rating | mount | 8048 | 8188 | 5000 | |
| SearchBox | mount | 1390 | 1351 | 5000 | |
| Shimmer | mount | 2606 | 2628 | 5000 | |
| Slider | mount | 2010 | 2038 | 5000 | |
| SpinButton | mount | 5278 | 5337 | 5000 | |
| Spinner | mount | 483 | 475 | 5000 | |
| SplitButton | mount | 3280 | 3388 | 5000 | |
| Stack | mount | 547 | 570 | 5000 | |
| StackWithIntrinsicChildren | mount | 2533 | 2360 | 5000 | |
| StackWithTextChildren | mount | 5609 | 5446 | 5000 | |
| SwatchColorPicker | mount | 12281 | 12035 | 5000 | |
| TagPicker | mount | 2865 | 2817 | 5000 | |
| TeachingBubble | mount | 101485 | 101940 | 5000 | |
| Text | mount | 471 | 438 | 5000 | |
| TextField | mount | 1521 | 1534 | 5000 | |
| ThemeProvider | mount | 1235 | 1248 | 5000 | |
| ThemeProvider | virtual-rerender | 690 | 645 | 5000 | |
| ThemeProvider | virtual-rerender-with-unmount | 1927 | 2016 | 5000 | |
| Toggle | mount | 825 | 872 | 5000 | |
| buttonNative | mount | 126 | 129 | 5000 |
| wrapper = mount(<ColorPicker color="#abcdef" />); | ||
| expect(wrapper.getDOMNode().getAttribute('aria-label')).toBe( | ||
| const { getAllByRole, rerender } = render(<ColorPicker color="#abcdef" />); | ||
| expect(getAllByRole('group')[0].getAttribute('aria-label')).toBe( |
There was a problem hiding this comment.
Is group just the wrapper element? If so I think something like container.firstElementChild might be better.
There was a problem hiding this comment.
Just made the change for this but just wondering, what makes container.firstElementChild better than getAllByRole('group')[0] in this case?
There was a problem hiding this comment.
Oh sorry, I was thinking "group" was some implied role, but now I see it's explicitly on the root element of the ColorPicker--so that's probably fine to get it by that role if you add a comment about what it is. (Or just leave it if you don't want to bother reverting changes. Sorry about the confusion, I should have looked at the component code!)
There was a problem hiding this comment.
Ah no worries! Just reverted back to getting by role and declaring it in a colorPickerRoot variable for clarity.
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
Current Behavior
ColorPickertests don't use testing-libraryNew Behavior
ColorPickerto fully use testing-libraryRelated Issue(s)
part of #21663