-
Notifications
You must be signed in to change notification settings - Fork 516
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.
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
fix(selector): root classname is applied twice #2423
Conversation
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
Deploy preview ready! Built with commit 8b5d8fa https://deploy-preview-2423--algolia-instantsearch.netlify.com |
Deploy preview ready! Built with commit 4274d9b https://deploy-preview-2423--algolia-instantsearch.netlify.com |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you need to update the snapshots I think
I need push them 🤣 |
Good thing that we have argos :D |
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 :/
…ch.js into fix/select-css-class
The select was used but not implemented. The change is expected on argos. |
@@ -2,7 +2,7 @@ | |||
|
|||
exports[`Selector should render <Selector/> with numbers 1`] = ` | |||
<select | |||
className="custom-root" | |||
className={undefined} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we update this test to not have undefined inside it? And check if select css class is actually read?
[autoHideContainer=false] | ||
})`; | ||
|
||
/** | ||
* @typedef {Object} SortByWidgetCssClasses | ||
* @property {string|string[]} [root] CSS classes added to the parent `<select>`. | ||
* @property {string|string[]} [root] CSS classes added to the outer `<div>`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
4496076
to
4274d9b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
The implementation of the React component
Selector
was applying theroot
cssclasses that were already applied by the headerfooter. In order to
prevent regressions and let user customize the , this patch adds a new key to customize cssClasses in Selector bases widgets (select), and this new key has the root value to avoid breaking the current implementations. Fix #2396 #2397