Skip to content

Commit 44dca11

Browse files
authored
fix(selector): root classname is applied twice (#2423)
* fix(selector): root classname is applied twice The implementation of the React `selector` was applying the root css classname that is already applied by the headerfooter. In order to prevent regressions and let user customize the <select>, this patch adds a new key to customize cssClasses in Selector bases widgets, and this new key has the root value to avoid breaking the current implementations. Fix #2396 #2397 * chore(test): update snapshot * chore(functional-tests): add form-control using select The new cssClasses key so that we check that all 3 can have custom css classes. Interestingly the key was used but not implemented already :/ * chore(test): update test to validate the usage of the new key
1 parent ade3993 commit 44dca11

File tree

11 files changed

+36
-9
lines changed

11 files changed

+36
-9
lines changed

functional-tests/app/app.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,9 @@ search.addWidget(
299299
{ label: 'Top 100', value: 9901 },
300300
{ label: 'Top 500', value: 9501 },
301301
],
302+
cssClasses: {
303+
select: 'form-control',
304+
},
302305
})
303306
);
304307

src/components/Selector.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ export class RawSelector extends React.Component {
1818

1919
return (
2020
<select
21-
className={this.props.cssClasses.root}
21+
className={this.props.cssClasses.select}
2222
onChange={this.handleChange}
2323
value={currentValue}
2424
>
@@ -42,6 +42,10 @@ RawSelector.propTypes = {
4242
PropTypes.string,
4343
PropTypes.arrayOf(PropTypes.string),
4444
]),
45+
select: PropTypes.oneOfType([
46+
PropTypes.string,
47+
PropTypes.arrayOf(PropTypes.string),
48+
]),
4549
item: PropTypes.oneOfType([
4650
PropTypes.string,
4751
PropTypes.arrayOf(PropTypes.string),

src/components/__tests__/Selector-test.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ describe('Selector', () => {
99
setValue: () => {},
1010
cssClasses: {
1111
root: 'custom-root',
12+
select: 'custom-select',
1213
item: 'custom-item',
1314
},
1415
options: [
@@ -26,6 +27,7 @@ describe('Selector', () => {
2627
setValue: () => {},
2728
cssClasses: {
2829
root: 'custom-root',
30+
select: 'custom-select',
2931
item: 'custom-item',
3032
},
3133
options: [

src/components/__tests__/__snapshots__/Selector-test.js.snap

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
exports[`Selector should render <Selector/> with numbers 1`] = `
44
<select
5-
className="custom-root"
5+
className="custom-select"
66
onChange={[Function]}
77
value={10}
88
>
@@ -23,7 +23,7 @@ exports[`Selector should render <Selector/> with numbers 1`] = `
2323

2424
exports[`Selector should render <Selector/> with strings 1`] = `
2525
<select
26-
className="custom-root"
26+
className="custom-select"
2727
onChange={[Function]}
2828
value="index-a"
2929
>

src/widgets/hits-per-page-selector/__tests__/__snapshots__/hits-per-page-selector-test.js.snap

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ exports[`hitsPerPageSelector() calls twice ReactDOM.render(<Selector props />, c
66
Object {
77
"item": "ais-hits-per-page-selector--item custom-item",
88
"root": "ais-hits-per-page-selector custom-root cx",
9+
"select": "ais-hits-per-page-selector custom-select",
910
}
1011
}
1112
currentValue={10}

src/widgets/hits-per-page-selector/__tests__/hits-per-page-selector-test.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ describe('hitsPerPageSelector()', () => {
3737
];
3838
cssClasses = {
3939
root: ['custom-root', 'cx'],
40+
select: 'custom-select',
4041
item: 'custom-item',
4142
};
4243
widget = hitsPerPageSelector({ container, items, cssClasses });

src/widgets/hits-per-page-selector/hits-per-page-selector.js

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,13 +35,14 @@ const usage = `Usage:
3535
hitsPerPageSelector({
3636
container,
3737
items,
38-
[ cssClasses.{root,item}={} ],
38+
[ cssClasses.{root,select,item}={} ],
3939
[ autoHideContainer=false ]
4040
})`;
4141

4242
/**
4343
* @typedef {Object} HitsPerPageSelectorCSSClasses
44-
* @property {string|string[]} [root] CSS classes added to the parent `<select>`.
44+
* @property {string|string[]} [root] CSS classes added to the outer `<div>`.
45+
* @property {string|string[]} [select] CSS classes added to the parent `<select>`.
4546
* @property {string|string[]} [item] CSS classes added to each `<option>`.
4647
*/
4748

@@ -94,6 +95,9 @@ export default function hitsPerPageSelector(
9495

9596
const cssClasses = {
9697
root: cx(bem(null), userCssClasses.root),
98+
// We use the same class to avoid regression on existing website. It needs to be replaced
99+
// eventually by `bem('select')
100+
select: cx(bem(null), userCssClasses.select),
97101
item: cx(bem('item'), userCssClasses.item),
98102
};
99103

src/widgets/numeric-selector/__tests__/numeric-selector-test.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ describe('numericSelector()', () => {
2525
options = [{ value: 1, label: 'first' }, { value: 2, label: 'second' }];
2626
cssClasses = {
2727
root: ['custom-root', 'cx'],
28+
select: 'custom-select',
2829
item: 'custom-item',
2930
};
3031
widget = numericSelector({
@@ -37,6 +38,7 @@ describe('numericSelector()', () => {
3738
shouldAutoHideContainer: false,
3839
cssClasses: {
3940
root: 'ais-numeric-selector custom-root cx',
41+
select: 'ais-numeric-selector custom-select',
4042
item: 'ais-numeric-selector--item custom-item',
4143
},
4244
currentValue: 1,

src/widgets/numeric-selector/numeric-selector.js

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ const usage = `Usage: numericSelector({
3030
container,
3131
attributeName,
3232
options,
33-
cssClasses.{root,item},
33+
cssClasses.{root,select,item},
3434
autoHideContainer
3535
})`;
3636

@@ -42,7 +42,8 @@ const usage = `Usage: numericSelector({
4242

4343
/**
4444
* @typedef {Object} NumericSelectorCSSClasses
45-
* @property {string|string[]} [root] CSS classes added to the parent `<select>`.
45+
* @property {string|string[]} [root] CSS classes added to the outer `<div>`.
46+
* @property {string|string[]} [select] CSS classes added to the parent `<select>`.
4647
* @property {string|string[]} [item] CSS classes added to each `<option>`.
4748
*/
4849

@@ -98,6 +99,9 @@ export default function numericSelector({
9899

99100
const cssClasses = {
100101
root: cx(bem(null), userCssClasses.root),
102+
// We use the same class to avoid regression on existing website. It needs to be replaced
103+
// eventually by `bem('select')
104+
select: cx(bem(null), userCssClasses.select),
101105
item: cx(bem('item'), userCssClasses.item),
102106
};
103107

src/widgets/sort-by-selector/__tests__/sort-by-selector-test.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ describe('sortBySelector()', () => {
4343
];
4444
cssClasses = {
4545
root: ['custom-root', 'cx'],
46+
select: 'custom-select',
4647
item: 'custom-item',
4748
};
4849
widget = sortBySelector({ container, indices, cssClasses });
@@ -70,6 +71,7 @@ describe('sortBySelector()', () => {
7071
shouldAutoHideContainer: false,
7172
cssClasses: {
7273
root: 'ais-sort-by-selector custom-root cx',
74+
select: 'ais-sort-by-selector custom-select',
7375
item: 'ais-sort-by-selector--item custom-item',
7476
},
7577
currentValue: 'index-a',

0 commit comments

Comments
 (0)