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

[DC-875] Debounce Text Input and Slider #4714

Merged
merged 11 commits into from
Mar 20, 2024
Merged

[DC-875] Debounce Text Input and Slider #4714

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
1d63c5e
debounce text input and slider
evansuslovich Mar 13, 2024
9afac8e
debounce input
evansuslovich Mar 13, 2024
143a2e5
remove unused useCallBackg
evansuslovich Mar 18, 2024
0e3b0bb
debounce with useRef
evansuslovich Mar 19, 2024
1833a6c
added back a comment
evansuslovich Mar 19, 2024
897471b
change from debounceGetCriteriaCount to debounceSetCriteriaCount
evansuslovich Mar 19, 2024
2a2a7b3
remove mock
evansuslovich Mar 20, 2024
80f619c
renamed debounce functions and removed from dependencies
evansuslovich Mar 20, 2024
d00a9ee
add back a space
evansuslovich Mar 20, 2024
06f1ad1
Merge branch 'dev' into es/dc-875-debounce-year-of-birth-slider
evansuslovich Mar 20, 2024
8223d3e
Merge branch 'es/dc-875-debounce-year-of-birth-slider' of https://git…
evansuslovich Mar 20, 2024
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
7 changes: 7 additions & 0 deletions src/dataset-builder/CohortEditor.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,13 @@ jest.mock('src/libs/ajax/DataRepo', (): DataRepoExports => {
};
});

evansuslovich marked this conversation as resolved.
Show resolved Hide resolved
jest.mock('src/components/input', () => {
Copy link
Member

Choose a reason for hiding this comment

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

Is this mock still necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought that since the tests were passing they were needed but I removed the mock and the tests are still passing.

return {
...jest.requireActual('src/components/input'),
withDebouncedChange: (component) => component,
};
});

describe('CohortEditor', () => {
type CriteriaViewPropsOverrides = {
criteria: AnyCriteria;
Expand Down
10 changes: 6 additions & 4 deletions src/dataset-builder/CohortEditor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { div, h, h2, h3, strong } from 'react-hyperscript-helpers';
import { ButtonOutline, ButtonPrimary, GroupedSelect, Link, Select } from 'src/components/common';
import Slider from 'src/components/common/Slider';
import { icon } from 'src/components/icons';
import { NumberInput } from 'src/components/input';
import { NumberInput, withDebouncedChange } from 'src/components/input';
import { BuilderPageHeader } from 'src/dataset-builder/DatasetBuilderHeader';
import {
AnyCriteria,
Expand Down Expand Up @@ -47,6 +47,8 @@ type CriteriaViewProps = {

const addCriteriaText = 'Add criteria';

const DebouncedSlider = withDebouncedChange(Slider);
const DebouncedNumberInput = withDebouncedChange(NumberInput);
export const CriteriaView = (props: CriteriaViewProps) => {
const { datasetId, criteria, deleteCriteria, updateCriteria } = props;

Expand Down Expand Up @@ -128,7 +130,7 @@ export const CriteriaView = (props: CriteriaViewProps) => {
return h(Fragment, [
div([strong([`${criteria.option.name}:`]), ` ${criteria.low} - ${criteria.high}`]),
div({ style: { display: 'flex', alignItems: 'center' } }, [
h(NumberInput, {
h(DebouncedNumberInput, {
min: criteria.option.min,
max: criteria.high,
isClearable: false,
Expand All @@ -138,7 +140,7 @@ export const CriteriaView = (props: CriteriaViewProps) => {
'aria-label': `${criteria.option.name} low`,
style: numberInputStyles,
}),
h(Slider, {
h(DebouncedSlider, {
range: true,
value: [criteria.low, criteria.high],
min: criteria.option.min,
Expand All @@ -147,7 +149,7 @@ export const CriteriaView = (props: CriteriaViewProps) => {
style: { marginLeft: rangeSliderMargin },
ariaLabelForHandle: [`${criteria.option.name} low slider`, `${criteria.option.name} high slider`],
}),
h(NumberInput, {
h(DebouncedNumberInput, {
min: criteria.low,
max: criteria.option.max,
isClearable: false,
Expand Down
Loading