Clear methods to work with custom components #54
Conversation
| @@ -232,7 +232,7 @@ describe("The select event helpers", () => { | |||
| <Select {...defaultProps} defaultValue={OPTIONS[0]} isClearable /> | |||
| ); | |||
| expect(form).toHaveFormValues({ food: "chocolate" }); | |||
| await selectEvent.clearFirst(input); | |||
| await selectEvent.clearAll(input); | |||
There was a problem hiding this comment.
Can you revert this change? This test case tests the clearFirst utility.
| @@ -241,7 +241,7 @@ describe("The select event helpers", () => { | |||
| <Select {...defaultProps} defaultValue={OPTIONS[0]} isClearable /> | |||
| ); | |||
| expect(form).toHaveFormValues({ food: "chocolate" }); | |||
| await selectEvent.clearFirst(input); | |||
| await selectEvent.clearAll(input); | |||
There was a problem hiding this comment.
Same here, let’s leave the clearFirst
| const clearButton = container.querySelector('svg[aria-hidden="true"]')!; | ||
| // The "clear" button is constructed from the user-defined `${classNamePrefix}__multi-value__remove`. | ||
| // This is built from the internal util `cx` of react-select, so we take advantage of the attribute selector here. | ||
| const clearButton = container.querySelector('[class*="multi-value__remove"]')!; |
There was a problem hiding this comment.
We need to keep backwards compatibility with older react-select versions.
What would be the best way to achieve this?
Do we need to default to the svg selector of the new one doesn’t match anything?
|
Greetings all, I appreciate the work being done to support react-select and providing better testing tools. I wanted to stop by here to provide more clarification in regards to the ask, the issues, and a proposed solution. First, @AmirTugi made a request for react-select to provide a default `classNamePrefix. This is not something that the react-select team feels is in the best interest of all of its users, especially those who choose not to pollute their DOM with extra class name attributes for the sake of providing DOM attribute selectors. I believe this request was made with a misunderstanding of how the classNames and styles for react-select are generated. I have attempted to clarify this in the thread, but long story short, that bit is left to Emotion as the underlying CSS-in-JS solution. It should also be noted that those elements which return a className ending in the name of the related component are done via a label api which is fairly easy to implement. I think it would be beneficial to have "labels" provided for the interactive components (dropdownIndicator, clearIndicator, multiValueClear) and wouldn't add any extra new classes to the DOM (just simply appending defined string names to the end). 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')If this is a viable solution, we can have this considered as an addition to an already existing PR JedWatson/react-select#4342 which is being considered for next release. |
|
Hi @ebonow , this sounds great! |
|
@AmirTugi could you try testing your custom components on version 5.4.0 of this, I'm curious if my update to how the container is found will resolve this too. |
componentsfromreact-select- Change theclearFirstandclearAllmethods to work with the specific class names (${classNamePrefix}__multi-value_removeand${classNamePrefix}__clear-indicator).Important! this PR is blocked until we get answer/solution about those specific classes being added only if the user defined
classNamePrefixprop (here).