(react-slider) - Removing the imperative API #19401

czearing · 2021-08-13T22:56:23Z

Pull request checklist

Addresses an existing issue: Part of Slider Convergence #18886
Include a change request file using $ yarn change

Description of changes

Removing the imperative API from the Slider component and rewriting tests that used ref.curent.value.

…ix/slider-removing-imperative-api # Conflicts: # packages/react-slider/src/components/Slider/Slider.test.tsx

codesandbox-ci · 2021-08-13T23:04:15Z

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 57872bf:

Sandbox	Source
Fluent UI React Starter	Configuration

size-auditor · 2021-08-13T23:07:11Z

Asset size changes

Size Auditor did not detect a change in bundle size for any component!

Baseline commit: 5bd52b460354940ec2f54b043ebd5de62ad77fe1 (build)

fabricteam · 2021-08-13T23:12:53Z

📊 Bundle size report

_{🤖 This report was generated against 5bd52b460354940ec2f54b043ebd5de62ad77fe1}

fabricteam · 2021-08-13T23:24:07Z

Perf Analysis (`@fluentui/react`)

No significant results to display.

All results

Scenario	Render type	Master Ticks	PR Ticks	Iterations
Avatar	mount	901	920	5000
BaseButton	mount	892	891	5000
Breadcrumb	mount	2645	2693	1000
ButtonNext	mount	435	443	5000
Checkbox	mount	1538	1519	5000
CheckboxBase	mount	1286	1300	5000
ChoiceGroup	mount	4847	4715	5000
ComboBox	mount	1014	981	1000
CommandBar	mount	10464	10316	1000
ContextualMenu	mount	6354	6251	1000
DefaultButton	mount	1144	1144	5000
DetailsRow	mount	3777	3800	5000
DetailsRowFast	mount	3781	3757	5000
DetailsRowNoStyles	mount	3543	3621	5000
Dialog	mount	2180	2148	1000
DocumentCardTitle	mount	155	153	1000
Dropdown	mount	3264	3297	5000
FluentProviderNext	mount	7420	7479	5000
FocusTrapZone	mount	1849	1784	5000
FocusZone	mount	1791	1813	5000
IconButton	mount	1724	1735	5000
Label	mount	347	328	5000
Layer	mount	1774	1775	5000
Link	mount	478	477	5000
MakeStyles	mount	1848	1825	50000
MenuButton	mount	1494	1464	5000
MessageBar	mount	2013	2029	5000
Nav	mount	3287	3245	1000
OverflowSet	mount	1109	1091	5000
Panel	mount	2075	2084	1000
Persona	mount	838	839	1000
Pivot	mount	1409	1427	1000
PrimaryButton	mount	1311	1289	5000
Rating	mount	7708	7595	5000
SearchBox	mount	1344	1339	5000
Shimmer	mount	2547	2564	5000
Slider	mount	1968	1986	5000
SpinButton	mount	5011	5013	5000
Spinner	mount	425	433	5000
SplitButton	mount	3164	3184	5000
Stack	mount	505	507	5000
StackWithIntrinsicChildren	mount	1509	1511	5000
StackWithTextChildren	mount	4481	4472	5000
SwatchColorPicker	mount	10123	10232	5000
Tabs	mount	1454	1442	1000
TagPicker	mount	2569	2634	5000
TeachingBubble	mount	12063	11983	5000
Text	mount	429	430	5000
TextField	mount	1388	1412	5000
ThemeProvider	mount	1196	1189	5000
ThemeProvider	virtual-rerender	614	599	5000
Toggle	mount	838	811	5000
buttonNative	mount	112	108	5000

Perf Analysis (`@fluentui/react-northstar`)

Perf tests with no regressions

Scenario	Current PR Ticks	Baseline Ticks	Ratio
AccordionMinimalPerf.default	163	147	1.11:1
AvatarMinimalPerf.default	199	188	1.06:1
ButtonMinimalPerf.default	172	162	1.06:1
AnimationMinimalPerf.default	429	410	1.05:1
RadioGroupMinimalPerf.default	454	433	1.05:1
IconMinimalPerf.default	605	575	1.05:1
HeaderSlotsPerf.default	758	731	1.04:1
AttachmentMinimalPerf.default	158	154	1.03:1
CarouselMinimalPerf.default	474	462	1.03:1
FlexMinimalPerf.default	288	280	1.03:1
HeaderMinimalPerf.default	359	349	1.03:1
LabelMinimalPerf.default	387	376	1.03:1
PopupMinimalPerf.default	607	592	1.03:1
SegmentMinimalPerf.default	356	344	1.03:1
SkeletonMinimalPerf.default	359	350	1.03:1
TextMinimalPerf.default	343	332	1.03:1
AlertMinimalPerf.default	271	266	1.02:1
BoxMinimalPerf.default	342	334	1.02:1
ChatWithPopoverPerf.default	366	358	1.02:1
CheckboxMinimalPerf.default	2778	2723	1.02:1
DialogMinimalPerf.default	768	750	1.02:1
DividerMinimalPerf.default	359	352	1.02:1
InputMinimalPerf.default	1273	1251	1.02:1
ListCommonPerf.default	630	619	1.02:1
ProviderMergeThemesPerf.default	1697	1667	1.02:1
VideoMinimalPerf.default	630	618	1.02:1
ButtonSlotsPerf.default	547	544	1.01:1
ChatDuplicateMessagesPerf.default	285	283	1.01:1
DropdownMinimalPerf.default	3119	3093	1.01:1
EmbedMinimalPerf.default	4151	4127	1.01:1
ItemLayoutMinimalPerf.default	1206	1199	1.01:1
ListNestedPerf.default	544	537	1.01:1
MenuButtonMinimalPerf.default	1684	1667	1.01:1
PortalMinimalPerf.default	176	175	1.01:1
TableManyItemsPerf.default	1920	1895	1.01:1
CustomToolbarPrototype.default	3872	3832	1.01:1
TooltipMinimalPerf.default	1027	1013	1.01:1
CardMinimalPerf.default	552	551	1:1
DropdownManyItemsPerf.default	668	668	1:1
ListMinimalPerf.default	506	507	1:1
MenuMinimalPerf.default	855	855	1:1
RosterPerf.default	1144	1139	1:1
ProviderMinimalPerf.default	967	963	1:1
ReactionMinimalPerf.default	377	376	1:1
RefMinimalPerf.default	232	231	1:1
TableMinimalPerf.default	400	399	1:1
TreeMinimalPerf.default	789	787	1:1
AttachmentSlotsPerf.default	1053	1063	0.99:1
ButtonOverridesMissPerf.default	1703	1720	0.99:1
GridMinimalPerf.default	344	348	0.99:1
SplitButtonMinimalPerf.default	3801	3828	0.99:1
StatusMinimalPerf.default	672	677	0.99:1
ToolbarMinimalPerf.default	950	958	0.99:1
ChatMinimalPerf.default	640	650	0.98:1
DatepickerMinimalPerf.default	5500	5592	0.98:1
FormMinimalPerf.default	392	398	0.98:1
ImageMinimalPerf.default	366	373	0.98:1
LayoutMinimalPerf.default	362	370	0.98:1
SliderMinimalPerf.default	1542	1573	0.98:1
TextAreaMinimalPerf.default	495	505	0.98:1
TreeWith60ListItems.default	171	174	0.98:1
ListWith60ListItems.default	626	644	0.97:1
LoaderMinimalPerf.default	686	705	0.97:1

…ix/slider-removing-imperative-api

ecraig12345 · 2021-08-17T02:53:34Z

packages/react-slider/src/components/Slider/Slider.test.tsx

  it('handles (id) prop', () => {
    render(<Slider id="test_id" data-testid="test" />);
    const sliderRoot = screen.getByTestId('test');
    expect(sliderRoot.getAttribute('id')).toEqual('test_id');
  });

-  it('applies the (defaultValue) prop', () => {


Is there another way you can test the defaultValue prop? (unless it's still here and I'm missing it)

Was this really resolved? I don't see another test checking the defaultValue prop

Oops, I thought I added this. I just updated the pr with the test. It follows the same behavior as the applies the value prop test using aria-valuenow:

it('applies the (defaultValue) prop', () => { render(<Slider defaultValue={10} min={0} max={100} />); expect(screen.getByRole('slider').getAttribute('aria-valuenow')).toEqual('10'); });

packages/react-slider/src/components/Slider/Slider.test.tsx

…ix/slider-removing-imperative-api # Conflicts: # packages/react-slider/src/components/Slider/Slider.test.tsx # packages/react-slider/src/components/Slider/useSliderState.tsx

czearing added 3 commits August 13, 2021 15:23

Removing the imperative API and updating tests

49e48c2

Merge branch 'master' of https://github.com/microsoft/fluentui into f…

bd70920

…ix/slider-removing-imperative-api # Conflicts: # packages/react-slider/src/components/Slider/Slider.test.tsx

Updating the API

eeebdd1

czearing requested review from ecraig12345 and khmakoto August 13, 2021 22:56

czearing requested a review from a team as a code owner August 13, 2021 22:56

msft-fluent-ui-bot assigned smhigley Aug 13, 2021

msft-fluent-ui-bot added the Component: Slider label Aug 13, 2021

behowell mentioned this pull request Aug 16, 2021

Slider Convergence #18886

Closed

40 tasks

czearing added 2 commits August 16, 2021 14:08

Merge branch 'master' of https://github.com/microsoft/fluentui into f…

2ce76cf

…ix/slider-removing-imperative-api

Replacing internal value getter in the test with onChange

2044461

ecraig12345 reviewed Aug 17, 2021

View reviewed changes

czearing added 2 commits August 18, 2021 18:10

Merge branch 'master' of https://github.com/microsoft/fluentui into f…

f929a93

…ix/slider-removing-imperative-api # Conflicts: # packages/react-slider/src/components/Slider/Slider.test.tsx # packages/react-slider/src/components/Slider/useSliderState.tsx

Adding defaultValue test

46b6c80

khmakoto approved these changes Aug 20, 2021

View reviewed changes

Merge branch 'master' of https://github.com/microsoft/fluentui into f…

57872bf

…ix/slider-removing-imperative-api # Conflicts: # packages/react-slider/src/components/Slider/Slider.test.tsx # packages/react-slider/src/components/Slider/useSliderState.tsx

czearing merged commit d5d6048 into microsoft:master Aug 23, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(react-slider) - Removing the imperative API #19401

(react-slider) - Removing the imperative API #19401

czearing commented Aug 13, 2021 •

edited by ecraig12345

Loading

codesandbox-ci bot commented Aug 13, 2021 •

edited

Loading

size-auditor bot commented Aug 13, 2021 •

edited

Loading

fabricteam commented Aug 13, 2021 •

edited

Loading

fabricteam commented Aug 13, 2021 •

edited

Loading

ecraig12345 Aug 17, 2021

khmakoto Aug 19, 2021

czearing Aug 19, 2021

(react-slider) - Removing the imperative API #19401

(react-slider) - Removing the imperative API #19401

Conversation

czearing commented Aug 13, 2021 • edited by ecraig12345 Loading

Pull request checklist

Description of changes

codesandbox-ci bot commented Aug 13, 2021 • edited Loading

size-auditor bot commented Aug 13, 2021 • edited Loading

Asset size changes

fabricteam commented Aug 13, 2021 • edited Loading

📊 Bundle size report

fabricteam commented Aug 13, 2021 • edited Loading

Perf Analysis (@fluentui/react)

Perf Analysis (@fluentui/react-northstar)

ecraig12345 Aug 17, 2021

Choose a reason for hiding this comment

khmakoto Aug 19, 2021

Choose a reason for hiding this comment

czearing Aug 19, 2021

Choose a reason for hiding this comment

czearing commented Aug 13, 2021 •

edited by ecraig12345

Loading

codesandbox-ci bot commented Aug 13, 2021 •

edited

Loading

size-auditor bot commented Aug 13, 2021 •

edited

Loading

fabricteam commented Aug 13, 2021 •

edited

Loading

fabricteam commented Aug 13, 2021 •

edited

Loading

Perf Analysis (`@fluentui/react`)

Perf Analysis (`@fluentui/react-northstar`)