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

Fix mixed optgroups #1110

Closed
wants to merge 2 commits into from
Closed

Conversation

voidus
Copy link
Contributor

@voidus voidus commented Apr 5, 2023

Description

Fixes #615 basically.

It includes a working cypress test. I did not add unit tests, but I'd be happy to do so if you are planning to merge this.

Screenshots (if appropriate)

choices

Types of changes

  • Chore (tooling change or documentation change)
  • Refactor (non-breaking change which maintains existing functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

I'd call it a bug (see #615 ) but some might call it a new feature? Also minor refactorings.

Checklist

  • My code follows the code style of this project.
  • I have added new tests for the bug I fixed/the new feature I added.
  • I have modified existing tests for the bug I fixed/the new feature I added.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

I think it's worth a mention in the changelog though.

Closes Choices-js#615.

This pushes the conversion of OPTION/OPTGROUP elements to Choice objects
into the WrappedSelect class and unifies the code paths a little between
groups-present and groups-not-present.

Some work towards possibly fixing Choices-js#615
@voidus
Copy link
Contributor Author

voidus commented Apr 8, 2024

Wow I forgot about this PR.... anyone we could ping to move this forward?

@Xon Xon mentioned this pull request Aug 6, 2024
9 tasks
@Xon
Copy link
Collaborator

Xon commented Aug 6, 2024

Thanks! Implemented as part of #1166

@Xon Xon closed this Aug 6, 2024
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

Successfully merging this pull request may close these issues.

optgroup does not work when some options are nested and some are not
3 participants