-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Default value for classNamePrefix #4362
Comments
Greetings @AmirTugi , There are users who prefer to keep their styling within JS/JSX files as opposed to using stylesheets, so in the interest of keeping the DOM cleaner for those individuals, it's likely that The classNames you are referring to are generated by So in what way are they different? Let's examine the difference between your examples in the codesandbox. Let's start by looking at the difference in classNames in the Control components. Both have a className of Here you'll see that if a
So in this way, the classNames are intentionally meant to be obfuscated when a classNamePrefix is not identified, because the underlying logic to display a components changing UI state would no longer correspond to the initial className. I hope this makes sense and also why providing a default |
@AmirTugi regarding your question about targeting the indicator icons, these do not necessarily follow the same naming conventions, but I have compiled is a fairly thorough list of options you have when using classNamePrefix in this thread: These would apply to the styling for the indicators in case you were worried about switching icons. // applied to all indicators
[prefix]__indicator
// indicator container
[prefix]__indicators-container
// specific indicator components
[prefix]__clear-indicator
[prefix]__dropdown-indicator
[prefix]__loading-indicator
[prefix]__indicator-separator In regards to your use-case, I do believe that the selector targeting is a somewhat brittle implementation, so I do agree that this should provide some other means of selecting the clear indicator other than this existing querySelector // The "clear" button is the first svg element that is hidden to screen readers
const clearButton = container.querySelector('svg[aria-hidden="true"]')!; I'll think a bit more on how to achieve your desired behavior with custom components without requiring a PR. |
I think that keeping the DOM clear for users who do not use styling by classes should be either all or nothing. Regarding the test selectors - That's why I wanted to go by a firmer, more stable indicator. WDYT? |
@AmirTugi No worries. Let me explain a bit further. The styling of react-select is managed by Emotion. You can read more about its documentation in the provided link, but the gist of it is to provide a CSS-in-JS solution. Why? There are several other prominent CSS-in-JS libraries (Styled Components, Glam, etc...), but Emotion was chosen for react-select with This represents an issue for What goes unresolved by this is that a custom component can change any aspect of the expected DOM. Here is a real example that a user requested support for. codesandbox demo In that respect, I don't think that targeting by SVG path is any less fragile than assuming the clearable button in the first indicator. |
@AmirTugi I hope the above resolves the questions you have about what and why. In regards to resolving your use-case for react-select-event, there are a few approaches:
import Select, { components } from 'react-select';
let rseComponents = {};
for (const name in components) {
rseComponents[name] = (props) => {
const Component = componentProps => components[name](componentProps);
const innerProps = { ...props.innerProps, "data-select-component": name };
const newProps = { ...props, innerProps };
return <Component {...newProps} />;
};
}
const rseSelect = (props) => {
return <Select components={{ rseComponents }} {...props} />;
}; Not quite working yet, but should be a good start on the approach as you could then target by data attributes. I will say that I am not completely familiar with |
Greetings @AmirTugi , After some thought I have decided that it is might be more consistent to include generated classNames for the indicators. This would produce something to the effect of: <div aria-hidden="true" class="css-tlfecz-indicatorContainer">
<svg height="20" width="20" viewBox="0 0 20 20" aria-hidden="true" focusable="false" class="css-6q0nyr-Svg-dropdownIndicator">
<path d="M4.516 7.548c0.436-0.446 1.043-0.481 1.576 0l3.908 3.747 3.908-3.747c0.533-0.481 1.141-0.446 1.574 0 0.436 0.445 0.408 1.197 0 1.615-0.406 0.418-4.695 4.502-4.695 4.502-0.217 0.223-0.502 0.335-0.787 0.335s-0.57-0.112-0.789-0.335c0 0-4.287-4.084-4.695-4.502s-0.436-1.17 0-1.615z"></path>
</svg>
</div> With a className of const dropdownIndicator = selectEl.querySelector('class*=dropdownIndicator') We can continue this discussion in the As such though, the issue raised here is a request for creating a default for |
We will be closing this issue as this request would introduce an opinionated design decision. I understand that the intention is to develop a reliable way to target DOM nodes based on specific query selectors, but there are ongoing discussions about how to fully disentangle react-select from Emotion which would result in even these existing className hints from all classNames potentially being stripped out. I think a more solid approach to this would be to expose a a means to apply data attributes to the DOM nodes ie: I'll continue to keep an eye on this and hope to help out any way we can in providing better testing tools for the community. |
I couldn't find any information on why would
classNamePrefix
has no default value.When the user does not define
classNamePrefix
- the component-specific classes are not used (e.g -*__multi-value_remove
).Here's a sandbox of it.
This issue related somehow to #4123.
I think that those classes are great to have by default, and this means the user will need 0 config in order to get a specific selector for the internal components.
My use case is this PR for the
react-select-event
that allows us to find specific components not basing on the type of the component (svg
. What ifreact-select
will use a different icon in the future?).In order to resolve this, all we need is just add here the default value for the
classNamePrefix
(I suggestreact-select
).I can create the PR, and I just ant to know if there's any unseen backward compatibility effect on the user.
I created a PR for
react-select-event
that uses the internal class names, but it's awaiting this issue to resolve.The text was updated successfully, but these errors were encountered: