Skip to content
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

Dynamic selection of react-select parent breaks for empty select and with custom className #90

Closed
tupton opened this issue Apr 5, 2022 · 5 comments

Comments

@tupton
Copy link

tupton commented Apr 5, 2022

Empty selects have a ValueContainer whose class name ends with value-container. This gets picked up by the attribute selector matcher added in #86, which then causes value container to be erroneously treated as the react-select parent.

$0.closest('[class^="css-"][class$="-container"]')
<div class=​"css-1hwfws3 value-container">​…​</div>​

When a select has a value, this is no longer an issue because value-container--has-value becomes the last class in the list and the parent is correctly found:

$0.closest('[class^="css-"][class$="-container"]')
<div class=​"css-1pcexqc-container">​…​</div>​

However, this also breaks if you provide a className to the react-select component because react-select places it at the end.

It seems that we need to make the matcher for the parent only look at the first class in the class attribute value, but I'm unsure if that's possible with contains.

@tupton tupton changed the title Dynamic selection of react-select parent breaks for empty select Dynamic selection of react-select parent breaks for empty select and with custom className Apr 5, 2022
@romgain
Copy link
Owner

romgain commented Apr 5, 2022

@tupton apologies for this issue - would you mind pinning the library to version 5.3.0 until we find a solution?

@th3fallen would you mind taking a quick look at this? please let me know if you think you’d be able to contribute a fix for this, otherwise we might have to revert your PR 😢

@th3fallen
Copy link
Contributor

ahhh, it may be for the best to revert it for now, and I can revisit it and add tests for those cases.

@romgain
Copy link
Owner

romgain commented Apr 5, 2022

Thanks @th3fallen !

I don’t currently have access to a computer, but I’ll do that later today :)

@tupton
Copy link
Author

tupton commented Apr 5, 2022

@tupton apologies for this issue - would you mind pinning the library to version 5.3.0 until we find a solution?

No need to apologize. Thankfully, we already pin this (and every) dependency – dependabot wanted to update it and tests failed for that branch, which is how I discovered this issue.

We haven't updated in our development branch yet and can stay at 5.3.0 with no repercussions.

Thanks for taking a look!

@romgain
Copy link
Owner

romgain commented Apr 7, 2022

Done, this is reverted in v5.5.0

Thanks again for reporting this @tupton !

@tupton tupton closed this as completed Jan 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants