Skip to content

Migrate CategoryOptions dropdown to KMultiSelect #6092

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.

Sign up for GitHub

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

Merged
LianaHarris360 merged 5 commits into from
Aug 19, 2026
Merged

Migrate CategoryOptions dropdown to KMultiSelect #6092

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
abea701
Refactor CategoryOptions dropdown to KMultiSelect, keeping expanded m…
Prashant-thakur77 Aug 16, 2026
a461724
Rewrite CategoryOptions tests with Testing Library
Prashant-thakur77 Aug 16, 2026
11094ae
Move shared KMultiSelect strings to commonStrings
Prashant-thakur77 Aug 18, 2026
dcaabe2
done the changes asked in the review
Prashant-thakur77 Aug 19, 2026
1f96708
updated
Prashant-thakur77 Aug 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions contentcuration/contentcuration/frontend/shared/strings/commonStrings.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,40 @@ export const commonStrings = createTranslator('CommonStrings', {
message: 'Options',
context: 'Tooltip for the generic options menu icon',
},
openMenuAction: {
message: 'Open menu',
context: 'Accessible label for the button that opens a dropdown menu',
},
closeMenuAction: {
message: 'Close menu',
context: 'Accessible label for the button that closes a dropdown menu',
},
optionsClickableLabel: {
message: 'Options are clickable',
context: 'Announced to screen reader users when a list of selectable options appears',
},
allOptionsSelectedLabel: {
message: 'All options selected',
context: 'Announced when every option in a list is selected',
},
allOptionsDeselectedLabel: {
message: 'No options selected',
context: 'Announced when no options in a list are selected',
},
optionDeselectedLabel: {
message: 'Option deselected',
context: 'Announced when an option is removed from the selection',
},
partiallySelectedLabel: {
message: 'Partially selected',
context: 'Announced for an option when only some of the options under it are selected',
},
optionSelectedLabel: {
message: 'Selected {label}',
context: 'Announced when an option is selected. {label} is the name of the option',
},
optionRemovedLabel: {
message: 'Removed {label}',
context: 'Announced when an option is removed. {label} is the name of the option',
},
});
1 change: 1 addition & 0 deletions contentcuration/contentcuration/frontend/shared/strings/communityChannelsStrings.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,7 @@ export const communityChannelsStrings = createTranslator('CommunityChannelsStrin
message: 'New',
context: 'Label indicating the section for new notifications',
},
// TODO: clearAllAction should be moved to commonStrings.js in next major Studio release
clearAllAction: {
message: 'Clear all',
context: 'Action button to clear all notifications',
Expand Down
112 changes: 76 additions & 36 deletions contentcuration/contentcuration/frontend/shared/views/contentNodeFields/CategoryOptions.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,39 @@
<template>

<div>
<DropdownWrapper>
<KMultiSelect
v-if="!expanded"
:value="autocompleteValues"
:options="categoriesList"
itemValue="value"
itemText="text"
:label="translateMetadataString('category')"
:autoPromoteParent="false"
clearable
:noResultsText="$tr('noCategoryFoundText')"
:messages="messages"
@input="onKMultiSelectInput"
>
<template #chip="{ option, remove: removeChip }">
<span :ref="'category-chip-' + option.value">
<KChip
:text="option.text"

@rtibblesbot rtibblesbot Aug 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Resolved — addressed in the current code.

blocking: KChip gets :text and close but no removeLabel, so its close button falls back to KDS's hard-coded English (KChip/index.vue:107 — `Remove ${this.text}`). Every category chip's ✕ is announced as Remove Dance in every locale, and categoryOptions.spec.js:98 asserts that literal, pinning it in place.

Fix is one binding plus one new string. Since the KTooltip is trigger="hover", the full path is mouse-only today — passing it here covers keyboard and screen reader users too:

:removeLabel="removeCategoryAction$({ label: tooltipText(option.value) })"

@Prashant-thakur77 Prashant-thakur77 Aug 19, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

:removeLabel="$tr('removeCategory', { label: tooltipText(option.value) })"
close
@close="removeChip"

@rtibblesbot rtibblesbot Aug 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Resolved — addressed in the current code.

suggestion: KDS's default chip slot puts @mousedown.native.prevent on the KChip (KMultiSelectInput.vue:30); this override drops it. Without it, mousedown on the close button blurs the input, so onInputBlur schedules the rAF at KMultiSelect/index.vue:433 that can closeDropdown() and emit blur. onChipRemove refocuses on nextTick and normally wins the race — was chip removal checked with the dropdown open? Adding the modifier removes the question.

@Prashant-thakur77 Prashant-thakur77 Aug 19, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@mousedown.native.prevent
/>
</span>
<KTooltip
:reference="'category-chip-' + option.value"
:refs="$refs"
placement="top"
:text="tooltipText(option.value)"
/>
</template>
</KMultiSelect>

<DropdownWrapper v-if="expanded">
<template #default="{ attach, menuProps }">
<VAutocomplete
:value="autocompleteValues"
Expand All @@ -18,8 +50,8 @@
:menu-props="{
...menuProps,
zIndex: 4,
height: expanded ? 0 : 'auto',
maxHeight: expanded ? 0 : 300,
height: 0,

@rtibblesbot rtibblesbot Aug 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Resolved — addressed in the current code.

suggestion: With the VAutocomplete now behind v-if="expanded" and its menu pinned to height: 0 / maxHeight: 0, nothing inside the menu can render again. Unreachable now: #item (lines 89-105, with treeItemStyle/isSelected/onChange), #no-data (79-87), and the .parentOption rule at line 380 whose only user is line 92. The expanded branch still needs only #selection and :searchInput.sync.

@Prashant-thakur77 Prashant-thakur77 Aug 19, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done,removed

maxHeight: 0,
}"
:attach="attach"
@click:clear="$nextTick(() => removeAll())"
Expand All @@ -44,34 +76,6 @@
</div>
</VTooltip>
</template>

<template #no-data>
<VListTile v-if="categoryText && categoryText.trim()">
<VListTileContent>
<VListTileTitle>
{{ $tr('noCategoryFoundText', { text: categoryText.trim() }) }}
</VListTileTitle>
</VListTileContent>
</VListTile>
</template>

<template #item="{ item }">
<VListTile
:value="isSelected(item.value)"
:class="{ parentOption: !item.value.includes('.') }"
@mousedown.prevent
@click="onChange(item.value)"
>
<KCheckbox
:checked="isSelected(item.value)"
:label="item.text"
:value="item.value"
style="margin-top: 10px"
:style="treeItemStyle(item)"
:ripple="false"
/>
</VListTile>
</template>
</VAutocomplete>
</template>
</DropdownWrapper>
Expand Down Expand Up @@ -104,13 +108,17 @@
<script>

import camelCase from 'lodash/camelCase';
import KMultiSelect from 'kolibri-design-system/lib/candidate/multiselect/KMultiSelect';
import KChip from 'kolibri-design-system/lib/candidate/multiselect/KChip';
import { getSortedCategories } from 'shared/utils/helpers';
import { commonStrings } from 'shared/strings/commonStrings';
import { communityChannelsStrings } from 'shared/strings/communityChannelsStrings';
import DropdownWrapper from 'shared/views/form/DropdownWrapper';
import { constantsTranslationMixin, metadataTranslationMixin } from 'shared/mixins';

export default {
name: 'CategoryOptions',
components: { DropdownWrapper },
components: { KMultiSelect, KChip, DropdownWrapper },
mixins: [constantsTranslationMixin, metadataTranslationMixin],
props: {
/**
Expand Down Expand Up @@ -177,6 +185,34 @@
option.text.toLowerCase().includes(searchQuery),
);
},
messages() {
const {
openMenuAction$,
closeMenuAction$,
optionsClickableLabel$,
allOptionsSelectedLabel$,
allOptionsDeselectedLabel$,
optionDeselectedLabel$,
partiallySelectedLabel$,
optionSelectedLabel$,
optionRemovedLabel$,
} = commonStrings;
const { clearAllAction$ } = communityChannelsStrings;
return {
clearText: clearAllAction$,
open: openMenuAction$,
close: closeMenuAction$,
clickable: optionsClickableLabel$,
allOptionsSelected: allOptionsSelectedLabel$,
allOptionsDeselected: allOptionsDeselectedLabel$,
optionDeselected: optionDeselectedLabel$,
partiallySelected: partiallySelectedLabel$,
itemsSelected: ({ count }) => this.$tr('itemsSelected', { count }),
selected: optionSelectedLabel$,
removed: optionRemovedLabel$,
cleared: () => this.$tr('allCategoriesCleared'),
};
},
},
methods: {
treeItemStyle(item) {
Expand All @@ -201,6 +237,11 @@
removeAll() {
this.selected = {};
},
// Dropdown mode is only rendered when a single node is edited, so every
// selected category simply applies to all of nodeIds.
onKMultiSelectInput(newValues) {

@rtibblesbot rtibblesbot Aug 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Resolved — addressed in the current code.

suggestion: The ids.length !== this.nodeIds.length branch is unreachable. Dropdown mode renders only from DetailsTabView.vue:133 under v-if="oneSelected" (nodes.length === 1), and generateNestedNodesGetterSetterObject pushes one node id per node; the other call site (EditCategoriesModal) passes expanded. A hypothetical partial wouldn't survive the round trip anyway — the parent setter discards them (DetailsTabView.vue:601).

Suggest reducing to the reachable case and reinstating the carryover with the expanded-mode migration that actually needs it:

this.selected = Object.fromEntries(newValues.map(value => [value, this.nodeIds]));

@Prashant-thakur77 Prashant-thakur77 Aug 19, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

this.selected = Object.fromEntries(newValues.map(value => [value, this.nodeIds]));
},
tooltipText(optionId) {
const option = this.categoriesList.find(option => option.value === optionId);
if (!option) {
Expand Down Expand Up @@ -275,6 +316,9 @@
},
$trs: {
noCategoryFoundText: 'Category not found',
itemsSelected: '{count, plural, one {# category selected} other {# categories selected}}',

@rtibblesbot rtibblesbot Aug 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Resolved — addressed in the current code.

praise: Count kept inside the ICU plural rather than composed alongside the translated string — pluralisation and digit formatting stay under translator control.

allCategoriesCleared: 'All categories cleared',
removeCategory: 'Remove {label}',
},
};

Expand All @@ -283,10 +327,6 @@

<style lang="scss" scoped>

.parentOption:not(:first-child) {
border-top: 1px solid rgba(0, 0, 0, 0.12);
}

.checkbox-list-wrapper {
height: 250px;
overflow-y: auto;
Expand Down
Loading
Loading