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

feat(forms): back MultiThumbSlider with new useSlider hook #1431

Merged
merged 11 commits into from
Oct 18, 2022
Merged

feat(forms): back MultiThumbSlider with new useSlider hook #1431

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
ab0bb6f
Initial useSlider hook replacement
jzempel Sep 26, 2022
0d4394a
Update testing
jzempel Sep 27, 2022
2c5b83d
Add new uncontrolled and `jump` props
jzempel Sep 27, 2022
0532c84
Add uncontrolled and `jump` testing
jzempel Sep 27, 2022
bdc739c
Add container-slider dependency
jzempel Oct 10, 2022
8e20ec4
Uncontrolled does not make sense for this component without backing i…
jzempel Oct 10, 2022
4f4e330
Remove non-relevant documentation
jzempel Oct 10, 2022
2d7e9b9
Merge branch 'main' into jzempel/use-slider
jzempel Oct 10, 2022
5af67b8
Merge branch 'main' into jzempel/use-slider
jzempel Oct 10, 2022
e222a39
Upgrade to container-slider 0.1.1 with fixes for screenreader + keybo…
jzempel Oct 13, 2022
fabcbac
Merge branch 'main' into jzempel/use-slider
jzempel Oct 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions packages/forms/.size-snapshot.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,20 +1,20 @@
{
"index.cjs.js": {
"bundled": 142182,
"minified": 97726,
"gzipped": 17296
"bundled": 134969,
"minified": 95396,
"gzipped": 16628
},
"index.esm.js": {
"bundled": 132950,
"minified": 89467,
"gzipped": 16864,
"bundled": 125928,
"minified": 87330,
"gzipped": 16186,
"treeshaked": {
"rollup": {
"code": 73088,
"import_statements": 745
"code": 71113,
"import_statements": 790
},
"webpack": {
"code": 80882
"code": 78583
}
}
}
Expand Down
1 change: 1 addition & 0 deletions packages/forms/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
"types": "dist/typings/index.d.ts",
"dependencies": {
"@zendeskgarden/container-field": "^2.1.0",
"@zendeskgarden/container-slider": "^0.1.1",
"@zendeskgarden/container-utilities": "^0.7.0",
"lodash.debounce": "^4.0.8",
"polished": "^4.0.0",
Expand Down
46 changes: 41 additions & 5 deletions packages/forms/src/elements/MultiThumbRange.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,14 +203,14 @@ describe('MultiThumbRange', () => {
expect(thumb).toHaveAttribute('aria-valuemin', '0');
expect(thumb).toHaveAttribute('aria-valuemax', '75');
expect(thumb).toHaveAttribute('aria-valuenow', '15');
expect(thumb).toHaveAttribute('aria-valuetext', '15');
expect(thumb).toHaveAttribute('aria-label', '15');
});

it('removes thumb from tab order when disabled', () => {
const { getAllByTestId } = render(<MultiThumbRange disabled minValue={15} maxValue={75} />);
const thumb = getAllByTestId('thumb')[0];

expect(thumb).not.toHaveAttribute('tabIndex');
expect(thumb).toHaveAttribute('tabIndex', '-1');
});

it('applies correct style', () => {
Expand Down Expand Up @@ -291,6 +291,24 @@ describe('MultiThumbRange', () => {
expect(onChangeSpy).toHaveBeenCalledWith({ minValue: 20, maxValue: 75 });
});

it('jumps minValue up on PAGE_UP key', () => {
const { getAllByTestId } = render(
<MultiThumbRange minValue={15} maxValue={75} jump={10} onChange={onChangeSpy} />
);

fireEvent.keyDown(getAllByTestId('thumb')[0], { keyCode: KEY_CODES.PAGE_UP });
expect(onChangeSpy).toHaveBeenCalledWith({ minValue: 25, maxValue: 75 });
});

it('jumps minValue down on PAGE_DOWN key', () => {
const { getAllByTestId } = render(
<MultiThumbRange minValue={15} maxValue={75} jump={10} onChange={onChangeSpy} />
);

fireEvent.keyDown(getAllByTestId('thumb')[0], { keyCode: KEY_CODES.PAGE_DOWN });
expect(onChangeSpy).toHaveBeenCalledWith({ minValue: 5, maxValue: 75 });
});

it('sets minValue to min on HOME key', () => {
const { getAllByTestId } = render(
<MultiThumbRange minValue={15} maxValue={75} step={5} onChange={onChangeSpy} />
Expand Down Expand Up @@ -410,14 +428,14 @@ describe('MultiThumbRange', () => {
expect(thumb).toHaveAttribute('aria-valuemin', '15');
expect(thumb).toHaveAttribute('aria-valuemax', '100');
expect(thumb).toHaveAttribute('aria-valuenow', '75');
expect(thumb).toHaveAttribute('aria-valuetext', '75');
expect(thumb).toHaveAttribute('aria-label', '75');
});

it('removes thumb from tab order when disabled', () => {
const { getAllByTestId } = render(<MultiThumbRange disabled minValue={15} maxValue={75} />);
const thumb = getAllByTestId('thumb')[1];

expect(thumb).not.toHaveAttribute('tabindex');
expect(thumb).toHaveAttribute('tabindex', '-1');
});

it('applies correct style', () => {
Expand All @@ -438,7 +456,7 @@ describe('MultiThumbRange', () => {
const { getAllByTestId } = render(<MultiThumbRange minValue={50} maxValue={40} />);
const thumb = getAllByTestId('thumb')[1];

expect(thumb).toHaveStyle('left: 50px');
expect(thumb).toHaveStyle('left: 40px');
Copy link
Contributor

Choose a reason for hiding this comment

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

I wouldn't have expected this to change, @jzempel do you know why?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, in the new hook we picked a consistent approach for applying out-of-bounds logic – that is, start-to-end (the minimum will determine where the maximum can start rather than the other way around). The previous logic was inconsistent in MultiThumbRange.

});

it('applies correct style if maxValue is less than min', () => {
Expand Down Expand Up @@ -502,6 +520,24 @@ describe('MultiThumbRange', () => {
expect(onChangeSpy).toHaveBeenCalledWith({ minValue: 15, maxValue: 80 });
});

it('jumps maxValue up on PAGE_UP key', () => {
const { getAllByTestId } = render(
<MultiThumbRange minValue={15} maxValue={75} jump={10} onChange={onChangeSpy} />
);

fireEvent.keyDown(getAllByTestId('thumb')[1], { keyCode: KEY_CODES.PAGE_UP });
expect(onChangeSpy).toHaveBeenCalledWith({ minValue: 15, maxValue: 85 });
});

it('jumps maxValue down on PAGE_DOWN key', () => {
const { getAllByTestId } = render(
<MultiThumbRange minValue={15} maxValue={75} jump={10} onChange={onChangeSpy} />
);

fireEvent.keyDown(getAllByTestId('thumb')[1], { keyCode: KEY_CODES.PAGE_DOWN });
expect(onChangeSpy).toHaveBeenCalledWith({ minValue: 15, maxValue: 65 });
});

it('sets maxValue to minValue on HOME key', () => {
const { getAllByTestId } = render(
<MultiThumbRange minValue={15} maxValue={75} step={5} onChange={onChangeSpy} />
Expand Down
Loading