Skip to content

Commit c9cc3b4

Browse files
authoredAug 15, 2023
Merge pull request #1788 from victoria-dos/fix-checkbox-filter
Fix CheckboxFilter
2 parents 9eda19e + 6a47dc6 commit c9cc3b4

File tree

10 files changed

+99
-57
lines changed

10 files changed

+99
-57
lines changed
 

‎package-lock.json

+63-19
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@
6262
"@octokit/rest": "^16.43.2",
6363
"@openshift/dynamic-plugin-sdk-webpack": "^3.0.0",
6464
"@patternfly/quickstarts": "^2.3.1",
65-
"@patternfly/react-core": "^4.247.1",
65+
"@patternfly/react-core": "^4.276.1",
6666
"@patternfly/react-icons": "^4.92.5",
6767
"@patternfly/react-table": "^4.110.22",
6868
"@testing-library/dom": "^8.18.1",

‎packages/components/src/ConditionalFilter/CheckboxFilter.tsx

+10-13
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
1-
import React, { Fragment, useEffect, useRef, useState } from 'react';
1+
import React, { Fragment, useEffect, useState } from 'react';
22
import { Select, SelectOption, SelectVariant } from '@patternfly/react-core';
3-
import isEqual from 'lodash/isEqual';
43
import omit from 'lodash/omit';
54
import TextFilter, { FilterItem, FilterValue, isFilterValue } from './TextFilter';
65

@@ -30,29 +29,27 @@ export interface CheckboxFilterProps {
3029
*/
3130
const CheckboxFilter: React.FunctionComponent<CheckboxFilterProps> = ({
3231
items = [],
33-
value = [],
32+
value,
3433
onChange = () => undefined,
3534
isDisabled = false,
3635
...props
3736
}) => {
3837
const { placeholder, className } = props;
3938
const [isExpanded, setExpanded] = useState(false);
4039
const [selected, setSelected] = useState<(string | FilterValue)[]>([]);
41-
const prevSelected = useRef(selected);
42-
43-
const changeSelected = (value: (string | FilterValue)[]) => {
44-
prevSelected.current = selected;
45-
setSelected(value);
46-
};
4740

4841
useEffect(() => {
49-
!isEqual(prevSelected.current, value) && value && changeSelected(value as (string | FilterValue)[]);
50-
}, [selected, value]);
42+
if (value === undefined) {
43+
setSelected([]);
44+
} else {
45+
setSelected(Array.isArray(value) ? value : ([value] as (string | FilterValue)[]));
46+
}
47+
}, [value]);
5148

5249
const calculateSelected = () =>
5350
Array.from(
5451
new Set([
55-
...(value && (value as (string | FilterValue)[]).length > 0 && value.constructor === Array
52+
...(value && (value as (string | FilterValue)[]).length > 0 && Array.isArray(value)
5653
? value.map((item) => {
5754
return isFilterValue(item) ? item.value : item;
5855
})
@@ -66,7 +63,7 @@ const CheckboxFilter: React.FunctionComponent<CheckboxFilterProps> = ({
6663
newSelection = newSelection.includes(selection) ? newSelection.filter((item) => item !== selection) : [...newSelection, selection];
6764

6865
onChange?.(event, newSelection, selection);
69-
changeSelected(newSelection);
66+
setSelected(newSelection);
7067
};
7168

7269
return (

‎packages/components/src/ConditionalFilter/TextFilter.tsx

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { SearchIcon } from '@patternfly/react-icons';
44
import './conditional-filter.scss';
55

66
export function isFilterValue(item: string | FilterValue): item is FilterValue {
7-
return (item as FilterValue).value !== undefined;
7+
return (item as FilterValue)?.value !== undefined;
88
}
99
export interface FilterItem {
1010
/** Item label. */

‎packages/components/src/ConditionalFilter/__snapshots__/CheckboxFilter.test.js.snap

+12-12
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,10 @@ exports[`Checkbox render should render correctly - disabled 1`] = `
1111
<button
1212
aria-expanded="false"
1313
aria-label="Options menu"
14-
aria-labelledby=" pf-select-toggle-id-0"
14+
aria-labelledby=" pf-select-toggle-id-1"
1515
class="pf-c-select__toggle pf-m-disabled"
1616
disabled=""
17-
id="pf-select-toggle-id-0"
17+
id="pf-select-toggle-id-1"
1818
type="button"
1919
>
2020
<div
@@ -87,9 +87,9 @@ exports[`Checkbox render should render correctly placeholder 1`] = `
8787
<button
8888
aria-expanded="false"
8989
aria-label="Options menu"
90-
aria-labelledby=" pf-select-toggle-id-7"
90+
aria-labelledby=" pf-select-toggle-id-11"
9191
class="pf-c-select__toggle"
92-
id="pf-select-toggle-id-7"
92+
id="pf-select-toggle-id-11"
9393
type="button"
9494
>
9595
<div
@@ -134,9 +134,9 @@ exports[`Checkbox render should render correctly with items 1`] = `
134134
<button
135135
aria-expanded="false"
136136
aria-label="Options menu"
137-
aria-labelledby=" pf-select-toggle-id-1"
137+
aria-labelledby=" pf-select-toggle-id-3"
138138
class="pf-c-select__toggle"
139-
id="pf-select-toggle-id-1"
139+
id="pf-select-toggle-id-3"
140140
type="button"
141141
>
142142
<div
@@ -179,9 +179,9 @@ exports[`Checkbox render should render correctly with items and default object v
179179
<button
180180
aria-expanded="false"
181181
aria-label="Options menu"
182-
aria-labelledby=" pf-select-toggle-id-5"
182+
aria-labelledby=" pf-select-toggle-id-7"
183183
class="pf-c-select__toggle"
184-
id="pf-select-toggle-id-5"
184+
id="pf-select-toggle-id-7"
185185
type="button"
186186
>
187187
<div
@@ -233,9 +233,9 @@ exports[`Checkbox render should render correctly with items and default value 1`
233233
<button
234234
aria-expanded="false"
235235
aria-label="Options menu"
236-
aria-labelledby=" pf-select-toggle-id-3"
236+
aria-labelledby=" pf-select-toggle-id-5"
237237
class="pf-c-select__toggle"
238-
id="pf-select-toggle-id-3"
238+
id="pf-select-toggle-id-5"
239239
type="button"
240240
>
241241
<div
@@ -287,9 +287,9 @@ exports[`Checkbox render should render correctly with items and selected value 1
287287
<button
288288
aria-expanded="false"
289289
aria-label="Options menu"
290-
aria-labelledby=" pf-select-toggle-id-6"
290+
aria-labelledby=" pf-select-toggle-id-9"
291291
class="pf-c-select__toggle"
292-
id="pf-select-toggle-id-6"
292+
id="pf-select-toggle-id-9"
293293
type="button"
294294
>
295295
<div

‎packages/components/src/ConditionalFilter/__snapshots__/ConditionalFilter.test.js.snap

+4-4
Original file line numberDiff line numberDiff line change
@@ -216,9 +216,9 @@ exports[`ConditionalFilter render should render correctly checkbox with value ch
216216
<button
217217
aria-expanded="false"
218218
aria-label="Options menu"
219-
aria-labelledby=" pf-select-toggle-id-0"
219+
aria-labelledby=" pf-select-toggle-id-1"
220220
class="pf-c-select__toggle"
221-
id="pf-select-toggle-id-0"
221+
id="pf-select-toggle-id-1"
222222
type="button"
223223
>
224224
<div
@@ -435,9 +435,9 @@ exports[`ConditionalFilter render should render correctly radio filter with valu
435435
aria-expanded="false"
436436
aria-haspopup="listbox"
437437
aria-label="Options menu"
438-
aria-labelledby=" pf-select-toggle-id-1"
438+
aria-labelledby=" pf-select-toggle-id-2"
439439
class="pf-c-select__toggle"
440-
id="pf-select-toggle-id-1"
440+
id="pf-select-toggle-id-2"
441441
type="button"
442442
>
443443
<div

‎packages/components/src/PrimaryToolbar/__snapshots__/Actions.test.js.snap

+2-1
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ exports[`Actions - component API should update open 1`] = `
106106
</DropdownItem>,
107107
]
108108
}
109-
isFlipEnabled={false}
109+
isFlipEnabled={true}
110110
isGrouped={false}
111111
isOpen={true}
112112
isPlain={true}
@@ -120,6 +120,7 @@ exports[`Actions - component API should update open 1`] = `
120120
onToggle={[Function]}
121121
/>
122122
}
123+
zIndex={9999}
123124
>
124125
<div
125126
className="pf-c-dropdown pf-m-expanded"

‎packages/components/src/PrimaryToolbar/__snapshots__/PrimaryToolbar.test.js.snap

+2-2
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,7 @@ exports[`PrimaryToolbar should render with data full config 1`] = `
280280
}
281281
}
282282
variant="top"
283-
widgetId="pagination-options-menu"
283+
widgetId="options-menu"
284284
/>
285285
</ToolbarItem>
286286
</ToolbarContent>
@@ -555,7 +555,7 @@ exports[`PrimaryToolbar should render wrong actionsConfig 1`] = `
555555
}
556556
}
557557
variant="top"
558-
widgetId="pagination-options-menu"
558+
widgetId="options-menu"
559559
/>
560560
</ToolbarItem>
561561
</ToolbarContent>

‎packages/components/src/TagModal/__snapshots__/TableWithFilter.test.js.snap

+3-3
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ exports[`TableWithFilter should render with data and bulk select with calculate
200200
}
201201
}
202202
variant="bottom"
203-
widgetId="pagination-options-menu"
203+
widgetId="options-menu"
204204
/>
205205
</TableToolbar>
206206
</Fragment>
@@ -339,7 +339,7 @@ exports[`TableWithFilter should render with data and bulk select without calcula
339339
}
340340
}
341341
variant="bottom"
342-
widgetId="pagination-options-menu"
342+
widgetId="options-menu"
343343
/>
344344
</TableToolbar>
345345
</Fragment>
@@ -523,7 +523,7 @@ exports[`TableWithFilter should render with data and pagination 1`] = `
523523
}
524524
}
525525
variant="bottom"
526-
widgetId="pagination-options-menu"
526+
widgetId="options-menu"
527527
/>
528528
</TableToolbar>
529529
</Fragment>

‎packages/notifications/src/NotificationPagination/__snapshots__/NotificationPagination.test.js.snap

+1-1
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ exports[`Notification Pagination component should render correctly 1`] = `
5151
}
5252
}
5353
variant="bottom"
54-
widgetId="pagination-options-menu"
54+
widgetId="options-menu"
5555
/>
5656
</LevelItem>
5757
</Level>

0 commit comments

Comments
 (0)
Please sign in to comment.