Choices icon indicating copy to clipboard operation
Choices copied to clipboard

Allow deselect when renderSelectedChoices enabled

Open godismyjudge95 opened this issue 1 year ago • 2 comments

Description

Drafting this PR as I am not sure if you would want to follow a different approach. If this looks good I can try to add tests.

There have been a few requests to allow for deselecting items via the dropdown when the renderSelectedChoices option is enabled. #804 #884

I ran across this issue myself while needing to keep a multiselect box footprint minimal (it grows to be pretty large when selecting a few options).

This PR has 3 parts:m

  • highlight selected choices in the dropdown when renderSelectedChoices is enabled
  • allow clicking on a selected choice in the dropdown to deselect the choice
  • add a new renderItems boolean option to disable the "pills" in the multiselect input to allow one to only select/deselect via the dropdown

Screenshots (if appropriate)

Example of having both renderSelectedChoices: 'always' and renderItems: false

image

Types of changes

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

Checklist

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

godismyjudge95 avatar Oct 11 '24 15:10 godismyjudge95

Когда уже сделайте?!)

igorzharov avatar May 06 '25 04:05 igorzharov

Please ensure unit tests and e2e tests are passing.

All tests except the demo-page screenshot test are passing. I am not sure how to resolve that one? Do I need to generate a new screenshot?

  • An e2e test for each functionality change should also be added. Ideally one each for text, select-one and select-multiple even if it is testing it doesn't do anything.

I added a test for select-one and select-multiple. Not sure the text one is relevant in this case? Select one test ensures it does not show the deselect text as this PR does not enable that - although it could.

This change has a lot of unrelated code churn which complicates git blame. Please rebuild the PR without those changes to preserve git blame

Apologies, this PR was rushed just to see if you'd be open to the idea. The PR should now be much more concise.

Changes to .js files not matched in the .ts files. The .ts are the canonical source and the .js changes will be discard at the next build. There are significant changes not committed into the .ts

This is fixed now.

godismyjudge95 avatar May 12 '25 19:05 godismyjudge95