Repository navigation
[Remove Vuetify from Studio] Replace CountryField VAutocomplete with KDS KMultiSelect #6293
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
base: unstable
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,8 +31,19 @@ const makeStore = ({ offline = false } = {}) => | |
| online: !offline, | ||
| }, | ||
| }, | ||
| actions: { | ||
| register: mockRegisterAction, | ||
| modules: { | ||
| account: { | ||
| namespaced: true, | ||
| actions: { | ||
| register: mockRegisterAction, | ||
| }, | ||
| }, | ||
| policies: { | ||
| namespaced: true, | ||
| getters: { | ||
| getPolicyAcceptedData: () => () => ({}), | ||
| }, | ||
| }, | ||
| }, | ||
| }); | ||
|
|
||
|
|
@@ -102,20 +113,7 @@ describe('Create account page', () => { | |
| }); | ||
| }); | ||
|
|
||
| // NOTE: | ||
| // Full form submission tests are intentionally skipped here. | ||
| // | ||
| // The "locations" field uses CountryField, which internally uses VAutocomplete | ||
| // (a Vuetify component). VAutocomplete does not reliably update its v-model | ||
| // state when interacted with via Vue Testing Library's userEvent APIs, which | ||
| // prevents a fully user-centric submission flow from being exercised. | ||
| // | ||
| // The "source" field has been migrated to KSelect (KDS) and no longer has | ||
| // this limitation, but "locations" remains the blocker. | ||
| // | ||
| // These tests will be re-enabled once CountryField is migrated away from | ||
| // VAutocomplete as part of the ongoing Vuetify removal effort. | ||
| it.skip('creates an account when the user submits valid information', async () => { | ||
| it('creates an account when the user submits valid information', async () => { | ||
| await renderComponent(); | ||
|
|
||
| await userEvent.type(screen.getByLabelText(/first name/i), 'Test'); | ||
|
|
@@ -126,6 +124,12 @@ describe('Create account page', () => { | |
|
|
||
| await userEvent.click(screen.getByLabelText(/tagging content sources/i)); | ||
|
|
||
| await userEvent.type(screen.getByRole('combobox', { name: /select all that apply/i }), 'Kenya'); | ||
| await userEvent.click(await screen.findByRole('option', { name: 'Kenya' })); | ||
|
|
||
| await userEvent.click(screen.getByText(/select one/i)); | ||
| await userEvent.click(await screen.findByText(/learning equality website/i)); | ||
|
|
||
| await userEvent.click(screen.getByLabelText(/i have read and agree to terms of service/i)); | ||
|
|
||
| const finishButton = screen.getByRole('button', { name: /finish/i }); | ||
|
|
@@ -137,12 +141,14 @@ describe('Create account page', () => { | |
| await userEvent.click(finishButton); | ||
|
|
||
| await waitFor(() => { | ||
| expect(mockRegisterAction).toHaveBeenCalled(); | ||
| expect(mockRegisterAction).toHaveBeenCalledWith( | ||
| expect.anything(), | ||
| expect.objectContaining({ locations: 'Kenya' }), | ||
| ); | ||
| }); | ||
| }); | ||
|
|
||
| // Skipped for the same reason as above — CountryField (VAutocomplete) blocker | ||
| it.skip('shows an offline error when the user is offline', async () => { | ||
| it('shows an offline error when the user is offline', async () => { | ||
| await renderComponent({ offline: true }); | ||
|
|
||
| await userEvent.type(screen.getByLabelText(/first name/i), 'Test'); | ||
|
|
@@ -153,11 +159,19 @@ describe('Create account page', () => { | |
|
|
||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nitpick: If the offline test keeps its form steps, lines 154–168 duplicate lines 119–133. A |
||
| await userEvent.click(screen.getByLabelText(/tagging content sources/i)); | ||
|
|
||
| await userEvent.type(screen.getByRole('combobox', { name: /select all that apply/i }), 'Kenya'); | ||
| await userEvent.click(await screen.findByRole('option', { name: 'Kenya' })); | ||
|
|
||
| await userEvent.click(screen.getByText(/select one/i)); | ||
| await userEvent.click(await screen.findByText(/learning equality website/i)); | ||
|
|
||
| await userEvent.click(screen.getByLabelText(/i have read and agree to terms of service/i)); | ||
|
|
||
| const finishButton = screen.getByRole('button', { name: /finish/i }); | ||
| await userEvent.click(finishButton); | ||
|
|
||
| expect(await screen.findByText(/offline/i)).toBeInTheDocument(); | ||
| expect( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: This test passes without triggering the submit flow. The offline banner renders on mount ( Assert the banner and disabled Finish button immediately after rendering, then remove the form steps. Also, scope the message assertion to the banner rather than matching by text alone. |
||
| await screen.findByText(/please connect to the internet to create an account/i), | ||
| ).toBeInTheDocument(); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,57 +1,44 @@ | ||||||||||||||||||
| <template> | ||||||||||||||||||
|
|
||||||||||||||||||
| <DropdownWrapper> | ||||||||||||||||||
| <template #default="{ attach, menuProps }"> | ||||||||||||||||||
| <VAutocomplete | ||||||||||||||||||
| v-model="locations" | ||||||||||||||||||
| :items="options" | ||||||||||||||||||
| :label="label || $tr('locationLabel')" | ||||||||||||||||||
| :multiple="multiple" | ||||||||||||||||||
| :box="box" | ||||||||||||||||||
| item-value="id" | ||||||||||||||||||
| item-text="name" | ||||||||||||||||||
| :required="required" | ||||||||||||||||||
| :rules="rules" | ||||||||||||||||||
| :search-input.sync="searchInput" | ||||||||||||||||||
| :no-data-text="$tr('noCountriesFound')" | ||||||||||||||||||
| :chips="multiple" | ||||||||||||||||||
| :attach="attach" | ||||||||||||||||||
| :menuProps="menuProps" | ||||||||||||||||||
| clearable | ||||||||||||||||||
| v-bind="$attrs" | ||||||||||||||||||
| /> | ||||||||||||||||||
| </template> | ||||||||||||||||||
| </DropdownWrapper> | ||||||||||||||||||
| <KMultiSelect | ||||||||||||||||||
| v-model="locations" | ||||||||||||||||||
| :options="options" | ||||||||||||||||||
| :label="label || $tr('locationLabel')" | ||||||||||||||||||
| :multiple="multiple" | ||||||||||||||||||
| itemValue="id" | ||||||||||||||||||
| itemText="name" | ||||||||||||||||||
| :disabled="disabled" | ||||||||||||||||||
| :invalid="invalid" | ||||||||||||||||||
| :invalidText="$tr('locationRequiredMessage')" | ||||||||||||||||||
| :noResultsText="$tr('noCountriesFound')" | ||||||||||||||||||
| :appearanceOverrides="{ maxWidth: fullWidth ? '100%' : '500px' }" | ||||||||||||||||||
| :messages="messages" | ||||||||||||||||||
| clearable | ||||||||||||||||||
| @blur="touched = true" | ||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: Is |
||||||||||||||||||
| /> | ||||||||||||||||||
|
|
||||||||||||||||||
| </template> | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
||||||||||||||||||
| <script> | ||||||||||||||||||
|
|
||||||||||||||||||
| import KMultiSelect from 'kolibri-design-system/lib/candidate/multiselect/KMultiSelect'; | ||||||||||||||||||
| import countries from '../../utils/countries'; | ||||||||||||||||||
|
|
||||||||||||||||||
| import DropdownWrapper from 'shared/views/form/DropdownWrapper'; | ||||||||||||||||||
| import { commonStrings } from 'shared/strings/commonStrings'; | ||||||||||||||||||
| import { createMultiSelectMessages } from 'shared/utils/multiSelectMessages'; | ||||||||||||||||||
|
|
||||||||||||||||||
| export default { | ||||||||||||||||||
| name: 'CountryField', | ||||||||||||||||||
| components: { DropdownWrapper }, | ||||||||||||||||||
| // $attrs are rebound to a descendent component | ||||||||||||||||||
| inheritAttrs: false, | ||||||||||||||||||
| components: { KMultiSelect }, | ||||||||||||||||||
| props: { | ||||||||||||||||||
| value: { | ||||||||||||||||||
| type: [String, Array], | ||||||||||||||||||
| default() { | ||||||||||||||||||
| return []; | ||||||||||||||||||
| }, | ||||||||||||||||||
| default: null, | ||||||||||||||||||
| }, | ||||||||||||||||||
| required: { | ||||||||||||||||||
| type: Boolean, | ||||||||||||||||||
| default: false, | ||||||||||||||||||
| }, | ||||||||||||||||||
| box: { | ||||||||||||||||||
| type: Boolean, | ||||||||||||||||||
| default: true, | ||||||||||||||||||
| }, | ||||||||||||||||||
| multiple: { | ||||||||||||||||||
| type: Boolean, | ||||||||||||||||||
| default: true, | ||||||||||||||||||
|
|
@@ -65,26 +52,27 @@ | |||||||||||||||||
| type: Boolean, | ||||||||||||||||||
| default: false, | ||||||||||||||||||
| }, | ||||||||||||||||||
| disabled: { | ||||||||||||||||||
| type: Boolean, | ||||||||||||||||||
| default: false, | ||||||||||||||||||
| }, | ||||||||||||||||||
| }, | ||||||||||||||||||
| data() { | ||||||||||||||||||
| return { | ||||||||||||||||||
| searchInput: '', | ||||||||||||||||||
| touched: false, | ||||||||||||||||||
| }; | ||||||||||||||||||
| }, | ||||||||||||||||||
| computed: { | ||||||||||||||||||
| locations: { | ||||||||||||||||||
| get() { | ||||||||||||||||||
| return this.value; | ||||||||||||||||||
| // KMultiSelect expects an array in multiple mode and a string or null in single mode | ||||||||||||||||||
| if (this.multiple) { | ||||||||||||||||||
| return this.value || []; | ||||||||||||||||||
| } | ||||||||||||||||||
| return this.value || null; | ||||||||||||||||||
| }, | ||||||||||||||||||
| set(value) { | ||||||||||||||||||
| this.$emit('input', value); | ||||||||||||||||||
|
|
||||||||||||||||||
| // If selecting multiple countries, a chip is created for the selected item, | ||||||||||||||||||
| // so the input can be cleared. If selecting a single country, the search input | ||||||||||||||||||
| // becomes the selected country and should not be cleared to stay visible. | ||||||||||||||||||
| if (this.multiple) { | ||||||||||||||||||
| setTimeout(this.searchInputClear, 1); | ||||||||||||||||||
| } | ||||||||||||||||||
| }, | ||||||||||||||||||
| }, | ||||||||||||||||||
| options() { | ||||||||||||||||||
|
|
@@ -97,16 +85,24 @@ | |||||||||||||||||
| }; | ||||||||||||||||||
| }); | ||||||||||||||||||
| }, | ||||||||||||||||||
| rules() { | ||||||||||||||||||
| return [v => (!this.required || v.length ? true : this.$tr('locationRequiredMessage'))]; | ||||||||||||||||||
| invalid() { | ||||||||||||||||||
| const hasSelection = this.multiple ? this.locations.length > 0 : Boolean(this.locations); | ||||||||||||||||||
| return this.required && this.touched && !hasSelection; | ||||||||||||||||||
| }, | ||||||||||||||||||
| autocompleteMaxWidth() { | ||||||||||||||||||
| return this.fullWidth ? '100%' : '500px'; | ||||||||||||||||||
| }, | ||||||||||||||||||
| }, | ||||||||||||||||||
| methods: { | ||||||||||||||||||
| searchInputClear() { | ||||||||||||||||||
| this.searchInput = ''; | ||||||||||||||||||
| messages() { | ||||||||||||||||||
| const { | ||||||||||||||||||
| clearAction$, | ||||||||||||||||||
| optionRemovedLabel$, | ||||||||||||||||||
| countryItemsSelectedLabel$, | ||||||||||||||||||
| selectionsClearedLabel$, | ||||||||||||||||||
| } = commonStrings; | ||||||||||||||||||
| return createMultiSelectMessages({ | ||||||||||||||||||
| clearText: clearAction$, | ||||||||||||||||||
| itemsSelected: countryItemsSelectedLabel$, | ||||||||||||||||||
| // KMultiSelect passes a label in single mode and a count in multiple mode | ||||||||||||||||||
| cleared: ({ label, count }) => | ||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: No test covers the single-mode branch ( studio/contentcuration/contentcuration/frontend/shared/views/__tests__/languageDropdown.spec.js Lines 99 to 106 in 75ada9b
|
||||||||||||||||||
| this.multiple ? selectionsClearedLabel$({ count }) : optionRemovedLabel$({ label }), | ||||||||||||||||||
| }); | ||||||||||||||||||
| }, | ||||||||||||||||||
| }, | ||||||||||||||||||
| $trs: { | ||||||||||||||||||
|
|
@@ -117,17 +113,3 @@ | |||||||||||||||||
| }; | ||||||||||||||||||
|
|
||||||||||||||||||
| </script> | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
||||||||||||||||||
| <style lang="scss" scoped> | ||||||||||||||||||
|
|
||||||||||||||||||
| ::v-deep .v-select__selections { | ||||||||||||||||||
| width: calc(100% - 48px); // Account for clear icon | ||||||||||||||||||
| min-height: 0 !important; | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| .v-autocomplete { | ||||||||||||||||||
| max-width: v-bind('autocompleteMaxWidth'); | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| </style> | ||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.