Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -131,10 +131,7 @@

<!-- Location -->
<h2 class="section-header">{{ $tr('locationLabel') }}*</h2>
<CountryField
v-model="form.locations"
clearable
/>
<CountryField v-model="form.locations" />
<div
v-if="!valid && (!form.locations || !form.locations.length)"
:style="{ color: $themeTokens.error }"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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: () => () => ({}),
},
},
},
});

Expand Down Expand Up @@ -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');
Expand All @@ -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' }));

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.

suggestion: The submit test doesn't check that the selected country reaches register. It only asserts toHaveBeenCalled() (L144). That passes if locations is empty, holds the translated label, or holds an {id, name} object. Pin the payload:

expect(mockRegisterAction).toHaveBeenCalledWith(
  expect.anything(),
  expect.objectContaining({ locations: 'Kenya' }),
);

This also covers the AC "The value sent to the server stays the English name".


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 });
Expand All @@ -141,8 +145,7 @@ describe('Create account page', () => {
});
});

// 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');
Expand All @@ -153,11 +156,19 @@ 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 });
await userEvent.click(finishButton);

expect(await screen.findByText(/offline/i)).toBeInTheDocument();
expect(
await screen.findByText(/please connect to the internet to create an account/i),
).toBeInTheDocument();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@
<CountryField
ref="locationDropdown"
v-model="locationFilter"
:outline="false"
:multiple="false"
label="Target location"
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,6 @@
:disabled="!canBeEdited"
:label="countryLabel$()"
fullWidth
:hide-details="true"
/>
</KTransition>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,11 +133,7 @@
<div class="mb-1 mt-3">
<label>{{ $tr('targetRegionsLabel') }}</label>
</div>
<CountryField
v-model="location"
:box="false"
:menu-props="{ zIndex: 1, offsetY: true }"
/>
<CountryField v-model="location" />
<KTextbox
v-model="import_count"
:invalid="errors.import_count"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,4 +106,12 @@ export const commonStrings = createTranslator('CommonStrings', {
message: '{count, plural, one {Cleared # selection} other {Cleared # selections}}',
context: 'Announced when all selected languages are cleared from a language picker',
},
countryItemsSelectedLabel: {
message: '{count, plural, one {# country selected} other {# countries selected}}',
context: 'Announced with the number of countries currently selected in a country picker',
},
countrySelectionsClearedLabel: {

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.

nitpick: This string duplicates languageSelectionsClearedLabel. It doesn't name the item type, so translators get the same string twice. Replace both with one selectionsClearedLabel shared by the two pickers.

message: '{count, plural, one {Cleared # selection} other {Cleared # selections}}',
context: 'Announced when all selected countries are cleared from a country picker',
},
});
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"
/>

</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,
Expand All @@ -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() {
Expand All @@ -97,16 +85,26 @@
};
});
},
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$,
countrySelectionsClearedLabel$,
} = commonStrings;
return createMultiSelectMessages({
clearText: clearAction$,
itemsSelected: countryItemsSelectedLabel$,
// KMultiSelect passes a label in single mode and a count in multiple mode
cleared: ({ label, count }) =>
this.multiple
? countrySelectionsClearedLabel$({ count })
: optionRemovedLabel$({ label }),
});
},
},
$trs: {
Expand All @@ -117,17 +115,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>
Loading
Loading