Skip to content

Conversation

@SJost
Copy link

@SJost SJost commented Dec 11, 2023

fix multiSelectField being partial

Constructor OptionListGrouped could not be processed by function multiSelectField. This patch fixes this, using tags within the generated Html.


Before submitting your PR, check that you've:

  • Bumped the version number

After submitting your PR:

  • Update the Changelog.md file with a link to your PR
  • Check that CI passes (or if it fails, for reasons unrelated to your change, like CI timeouts)

Constructor OptionListGrouped could not be processed by function multiSelectField. This patch fixes this, using <optgroup> tags within the generated Html.
@schoettl
Copy link
Contributor

Oh, thanks for fixing this! I missed to handle that case when I introduced OptionListGrouped in 2021.

@SJost
Copy link
Author

SJost commented Dec 12, 2023

@schoettl Happy to help. Though I am not sure what to do with the remaining open instructions to update the Changelog.md? How do I know which version the PR is merged into, if at all?

@SJost
Copy link
Author

SJost commented Dec 12, 2023

Furthermore, in the version that I am actually using, I have an additional Maybe (SomeMessage site) argument that is used as the None Option, if isReq is False, since my users had problems with de-selecting.

I could add versions selectField' and multiSelectField' with this option, if desired, but I thought it might only dilute this PR.

@schoettl
Copy link
Contributor

Furthermore, in the version that I am actually using, I have an additional Maybe (SomeMessage site) argument that is used as the None Option, if isReq is False, since my users had problems with de-selecting.

You mean your users have problems to clear the selection in the multi select field, which is usually done in desktop browsers by Ctrl + click? TBH, I would leave this to the developer and not provide an extra implementation for that.

Regarding the open checkboxes, I think you can just add a note in the changelog for the new version you set.

And it looks to me as if CI would be started at approval of a maintainer. I think it has timed-out too often.

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.

2 participants