Allow deselect when renderSelectedChoices enabled #1221
Conversation
| @@ -54,7 +54,7 @@ export interface Templates { | |||
|
|
|||
| choiceGroup(options: TemplateOptions, group: GroupFull): HTMLDivElement; | |||
|
|
|||
| choice(options: TemplateOptions, choice: ChoiceFull, selectText: string, groupText?: string): HTMLDivElement; | |||
| choice(options: TemplateOptions, choice: ChoiceFull, selectText: string, deselectText: string, groupText?: string): HTMLDivElement; | |||
There was a problem hiding this comment.
Instead of an argument, please use renderItems from the config. This can be done by adding it as a picked field for the TemplateOptions type.
This avoids a breaking change to the function signature.
| text-align: right; | ||
| } | ||
| } | ||
| &[data-type*='select-multiple'] .#{$choices-selector}__item--selectable { | ||
| &.is-selected::after { | ||
| content: attr(data-deselect-text); |
There was a problem hiding this comment.
This CSS should be scoped to having [data-deselect-text] similar to how [data-select-text] has been done.
| @@ -5,5 +5,5 @@ import { ChoiceFull } from '../interfaces/choice-full'; | |||
| type MappedInputTypeToChoiceType<T extends string | InputChoice | InputGroup> = T extends InputGroup ? GroupFull : ChoiceFull; | |||
| export declare const coerceBool: (arg: unknown, defaultValue?: boolean) => boolean; | |||
| export declare const stringToHtmlClass: (input: string | string[] | undefined) => string[] | undefined; | |||
| export declare const mapInputToChoice: <T extends string | InputChoice | InputGroup>(value: T, allowGroup: boolean) => MappedInputTypeToChoiceType<T>; | |||
| export declare const mapInputToChoice: <T extends string | InputChoice | InputGroup>(value: T, allowGroup: boolean, allowRawString?: boolean) => MappedInputTypeToChoiceType<T>; | |||
There was a problem hiding this comment.
This function's signature has changed and the compiled chocies.js has this function change, but the changes aren't committed to the .ts files.
There was a problem hiding this comment.
Please ensure unit tests and e2e tests are passing.
- An e2e test for each functionality change should also be added. Ideally one each for
text,select-oneandselect-multipleeven if it is testing it doesn't do anything.
This change has a lot of unrelated code churn which complicates git blame. Please rebuild the PR without those changes to preserve git blame
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
Xon
added
feature
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
renderSelectedChoicesoption is enabled. #804 #884I 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
renderSelectedChoicesis enabledrenderItemsboolean option to disable the "pills" in the multiselect input to allow one to only select/deselect via the dropdownScreenshots (if appropriate)
Example of having both
renderSelectedChoices: 'always'andrenderItems: falseTypes of changes
Checklist