Skip to content

Commit

Permalink
fix(phone-number): review
Browse files Browse the repository at this point in the history
  • Loading branch information
aesteves60 authored and dpellier committed Sep 25, 2023
1 parent 55b0990 commit 4a0c71d
Show file tree
Hide file tree
Showing 13 changed files with 89 additions and 43 deletions.
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
enum ODS_PHONE_NUMBER_COUTRIE {
enum ODS_PHONE_NUMBER_COUNTRY_PRESET {
All = 'all'
}

export {
ODS_PHONE_NUMBER_COUTRIE,
ODS_PHONE_NUMBER_COUNTRY_PRESET,
};
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { ODS_COUNTRY_ISO_CODE, ODS_COUNTRY_ISO_CODES, ODS_LOCALE } from '@ovhcloud/ods-common-core';
import { ODS_PHONE_NUMBER_COUTRIE } from '../constants/phone-number-countries';
import { ODS_PHONE_NUMBER_COUNTRY_PRESET } from '../constants/phone-number-countries';
import { OsdsPhoneNumber } from '../osds-phone-number';
import { OdsPhoneNumberController } from './controller';
import countriesTranslationEn from '@ovhcloud/ods-common-core/src/i18n/countries/en.json'
Expand All @@ -13,6 +13,7 @@ class OdsPhoneNumberMock extends OsdsPhoneNumber {
emitChange = jest.fn();
emitFocus = jest.fn();
emitBlur = jest.fn();
parsedCountries = ODS_COUNTRY_ISO_CODES as ODS_COUNTRY_ISO_CODE[];
}

describe('spec:ods-phone-number-controller', () => {
Expand Down Expand Up @@ -63,6 +64,12 @@ describe('spec:ods-phone-number-controller', () => {
const isoCode = controller.getDefaultIsoCode();
expect(isoCode).toBe(ODS_COUNTRY_ISO_CODE.AD);
});

it('should get the first element of parsedCountries as isoCode', () => {
setup({ parsedCountries: [ODS_COUNTRY_ISO_CODE.CH, ODS_COUNTRY_ISO_CODE.AD, ODS_COUNTRY_ISO_CODE.AE]});
const isoCode = controller.getDefaultIsoCode();
expect(isoCode).toBe(ODS_COUNTRY_ISO_CODE.CH);
});
});

describe('methods:getDefaultLocale', () => {
Expand Down Expand Up @@ -99,7 +106,7 @@ describe('spec:ods-phone-number-controller', () => {

describe('methods:getCountriesList', () => {
it('should get all countries', () => {
setup({ countries: ODS_PHONE_NUMBER_COUTRIE.All });
setup({ countries: ODS_PHONE_NUMBER_COUNTRY_PRESET.All });
const countries = controller.getCountriesList();
expect(countries).toEqual(ODS_COUNTRY_ISO_CODES);
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type { OsdsPhoneNumber } from '../osds-phone-number';
import { ODS_COUNTRY_ISO_CODE, ODS_COUNTRY_ISO_CODES, ODS_LOCALE, ODS_LOCALES } from '@ovhcloud/ods-common-core';
import { ODS_PHONE_NUMBER_COUTRIE } from '../constants/phone-number-countries';
import { ODS_PHONE_NUMBER_COUNTRY_PRESET } from '../constants/phone-number-countries';
import countriesTranslationEn from '@ovhcloud/ods-common-core/src/i18n/countries/en.json'
import countriesTranslationFr from '@ovhcloud/ods-common-core/src/i18n/countries/fr.json'

Expand All @@ -21,13 +21,13 @@ class OdsPhoneNumberController {
return this.getValueOrNavigatorOrDefault({
value: this.component.isoCode,
list: ODS_COUNTRY_ISO_CODES,
defaultValue: ODS_COUNTRY_ISO_CODE.AD,
defaultValue: this.component.parsedCountries[0],
guard: this.isOdsCountryCode,
});
}

getCountriesList(): readonly ODS_COUNTRY_ISO_CODE[] {
if (this.component.countries === ODS_PHONE_NUMBER_COUTRIE.All) {
getCountriesList(): readonly ODS_COUNTRY_ISO_CODE[] | string {
if (this.component.countries === ODS_PHONE_NUMBER_COUNTRY_PRESET.All) {
return ODS_COUNTRY_ISO_CODES;
}
return this.component.countries || [];
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import type { ODS_COUNTRY_ISO_CODE, ODS_LOCALE } from "@ovhcloud/ods-common-core";
import { ODS_PHONE_NUMBER_COUTRIE } from "../constants/phone-number-countries";
import { ODS_PHONE_NUMBER_COUNTRY_PRESET } from "../constants/phone-number-countries";

interface OdsPhoneNumberAttribute {
/** Ability to clear the phone number value */
clearable?: boolean;
/** A specific subset of countries to display in the select instead of the whole list */
countries?: ODS_COUNTRY_ISO_CODE[] | ODS_PHONE_NUMBER_COUTRIE.All;
countries?: ODS_COUNTRY_ISO_CODE[] | ODS_PHONE_NUMBER_COUNTRY_PRESET.All | string;
disabled?: boolean;
/** Indicates if the phone number shows error or not */
error?: boolean;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { odsComponentAttributes2StringAttributes, odsStringAttributes2Str } from
import { DEFAULT_ATTRIBUTE } from './constants/default-attributes';
import { ODS_COUNTRY_ISO_CODE } from '@ovhcloud/ods-common-core';
import { odsSetE2eInterceptRequest } from '@ovhcloud/ods-common-stencil';
import { ODS_PHONE_NUMBER_COUTRIE } from './constants/phone-number-countries';
import { ODS_PHONE_NUMBER_COUNTRY_PRESET } from './constants/phone-number-countries';

describe('e2e:osds-phone-number', () => {
const baseAttribute = { value: '' };
Expand Down Expand Up @@ -83,7 +83,7 @@ describe('e2e:osds-phone-number', () => {
});

it('should display select because of countries all', async () => {
await setup({ attributes: { countries: ODS_PHONE_NUMBER_COUTRIE.All }, cbkInterceptorRequest: myCbk });
await setup({ attributes: { countries: ODS_PHONE_NUMBER_COUNTRY_PRESET.All }, cbkInterceptorRequest: myCbk });

expect(select).not.toBeNull();
expect(select).toHaveClass('hydrated');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

&__option {
display: flex;
align-items: center;

&__flag {
display: inline-block;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,13 @@ import { odsComponentAttributes2StringAttributes, odsStringAttributes2Str, odsUn
import { DEFAULT_ATTRIBUTE } from './constants/default-attributes';
import { OsdsPhoneNumber } from './osds-phone-number';
import { ODS_COUNTRY_ISO_CODE, ODS_COUNTRY_ISO_CODES, ODS_LOCALE } from '@ovhcloud/ods-common-core';
import { ODS_PHONE_NUMBER_COUTRIE } from './constants/phone-number-countries';
import { OdsPhoneNumberController } from './core/controller';
import { ODS_PHONE_NUMBER_COUNTRY_PRESET } from './constants/phone-number-countries';

describe('spec:osds-phone-number', () => {
const baseAttribute = { ariaLabel: '', forbiddenValues: [], value: '' };
let page: SpecPage;
let root: HTMLElement | undefined;
let instance: OsdsPhoneNumber;
let controller: OdsPhoneNumberController;
let select: HTMLElement;

afterEach(() => {
Expand Down Expand Up @@ -63,7 +61,7 @@ describe('spec:osds-phone-number', () => {
name: 'countries',
defaultValue: DEFAULT_ATTRIBUTE.countries,
newValue: [ODS_COUNTRY_ISO_CODE.FR, ODS_COUNTRY_ISO_CODE.GB],
value: ODS_PHONE_NUMBER_COUTRIE.All,
value: ODS_PHONE_NUMBER_COUNTRY_PRESET.All,
setup: (countries) => setup({ attributes: { countries } }),
...config,
exclude: [OdsUnitTestAttributeType.REFLECTED, OdsUnitTestAttributeType.MUTABLE],
Expand Down Expand Up @@ -133,14 +131,14 @@ describe('spec:osds-phone-number', () => {
it('should get countries list with default value & not display select', async () => {
await setup();
instance.handlerCountries();
expect(instance.countriesList).toEqual([]);
expect(instance.parsedCountries).toEqual([]);
expect(select).toBe(null);
});

it('should get countries list with all', async () => {
await setup({ attributes: { countries: ODS_PHONE_NUMBER_COUTRIE.All} });
await setup({ attributes: { countries: ODS_PHONE_NUMBER_COUNTRY_PRESET.All } });
instance.handlerCountries();
expect(instance.countriesList).toEqual(ODS_COUNTRY_ISO_CODES);
expect(instance.parsedCountries).toEqual(ODS_COUNTRY_ISO_CODES);
expect(select).toBeDefined();
});

Expand All @@ -149,9 +147,18 @@ describe('spec:osds-phone-number', () => {
await setup({ });
instance.countries = countries;
instance.handlerCountries();
expect(instance.countriesList).toEqual(countries);
expect(instance.parsedCountries).toEqual(countries);
expect(select).toBeDefined();
});

it('should parsed string coutries', async () => {
await setup();
instance.countries = JSON.stringify(["fr", "gb"]);
instance.handlerCountries();
expect(instance.parsedCountries).toEqual([ODS_COUNTRY_ISO_CODE.FR, ODS_COUNTRY_ISO_CODE.GB]);
expect(select).toBeDefined();
});

});

describe('methods:handlerLocale', () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import type { OdsPhoneNumberAttribute } from './interfaces/attributes';
import { ODS_COUNTRY_ISO_CODE, ODS_LOCALE } from '@ovhcloud/ods-common-core';
import { OdsLogger, ODS_COUNTRY_ISO_CODE, ODS_LOCALE } from '@ovhcloud/ods-common-core';
import { Component, Host, h, Prop, State, Watch } from '@stencil/core';
import { ODS_INPUT_TYPE } from '@ovhcloud/ods-component-input';
import { DEFAULT_ATTRIBUTE } from './constants/default-attributes';
import { OdsPhoneNumberController } from './core/controller';
import { ODS_THEME_COLOR_INTENT } from '@ovhcloud/ods-common-theming';
import { ODS_PHONE_NUMBER_COUTRIE } from './constants/phone-number-countries';
import { ODS_PHONE_NUMBER_COUNTRY_PRESET } from './constants/phone-number-countries';

/**
* @slot (unnamed) - Phone Number content
Expand All @@ -14,16 +14,17 @@ import { ODS_PHONE_NUMBER_COUTRIE } from './constants/phone-number-countries';
tag: 'osds-phone-number',
styleUrl: 'osds-phone-number.scss',
shadow: true,
assetsDirs: ['../../assets'],
})
export class OsdsPhoneNumber implements OdsPhoneNumberAttribute {
private logger = new OdsLogger('OsdsPhoneNumber');
controller = new OdsPhoneNumberController(this);
parsedCountries: ODS_COUNTRY_ISO_CODE[] = [];

/** @see OdsPhoneNumberAttribute.clearable */
@Prop({ reflect: true }) clearable?: boolean = DEFAULT_ATTRIBUTE.clearable;

/** @see OdsPhoneNumberAttribute.countries */
@Prop({ reflect: true, mutable: true }) countries?: ODS_COUNTRY_ISO_CODE[] | ODS_PHONE_NUMBER_COUTRIE.All = DEFAULT_ATTRIBUTE.countries;
@Prop({ reflect: true, mutable: true }) countries?: ODS_COUNTRY_ISO_CODE[] | ODS_PHONE_NUMBER_COUNTRY_PRESET.All | string = DEFAULT_ATTRIBUTE.countries;

/** @see OdsPhoneNumberAttribute.disabled */
@Prop({ reflect: true }) disabled?: boolean = DEFAULT_ATTRIBUTE.disabled;
Expand All @@ -42,14 +43,14 @@ export class OsdsPhoneNumber implements OdsPhoneNumberAttribute {

@State() i18nCountriesMap!: Map<string, { isoCode: string , name: string }>;

@State() countriesList: readonly ODS_COUNTRY_ISO_CODE[] = [];
@State() hasCountries: boolean = false;

componentWillLoad(): void {
// order matter
this.handlerCountries();
this.isoCode = this.controller.getDefaultIsoCode();
this.locale = this.controller.getDefaultLocale();
this.handlerLocale(this.locale);
this.handlerCountries();
}

@Watch('locale')
Expand All @@ -63,8 +64,22 @@ export class OsdsPhoneNumber implements OdsPhoneNumberAttribute {

@Watch('countries')
handlerCountries(): void {
this.hasCountries = Boolean(this.countries?.length);
this.countriesList = this.controller.getCountriesList();
const countriesList = this.controller.getCountriesList();
this.parseCountries(countriesList);
this.hasCountries = !!this.parsedCountries?.length;
}

private parseCountries(countries: readonly ODS_COUNTRY_ISO_CODE[] | string) {
if (typeof countries === 'string') {
try {
this.parsedCountries = JSON.parse(countries);
} catch (err) {
this.logger.warn('[OsdsPhoneNumber] countries string could not be parsed.');
this.parsedCountries = [];
}
} else {
this.parsedCountries = [...countries];
}
}

render() {
Expand All @@ -77,9 +92,9 @@ export class OsdsPhoneNumber implements OdsPhoneNumberAttribute {
disabled={this.disabled}
error={this.error}
value={this.isoCode}>
{ this.countriesList?.map((country) => <osds-select-option value={ country }>
{ this.parsedCountries?.map((country) => <osds-select-option value={ country }>
<div class="phone-number__select__option">
<osds-flag iso={country}></osds-flag>
<osds-flag iso={country} class="phone-number__select__option__flag"></osds-flag>
<osds-text>{ this.i18nCountriesMap?.get(country)?.name }</osds-text>
</div>
</osds-select-option>) }
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
export type { OdsPhoneNumberAttribute } from './interfaces/attributes';
export { OsdsPhoneNumber } from './osds-phone-number';
export { ODS_PHONE_NUMBER_COUTRIE } from './constants/phone-number-countries';
export { ODS_PHONE_NUMBER_COUNTRY_PRESET } from './constants/phone-number-countries';
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,9 @@ import GenericStyle from '@ovhcloud/ods-common-core/docs/generic-style.mdx';


### With specific countries
<osds-phone-number countries="['fr', 'gb']" value="+33647652334"></osds-phone-number>
<osds-phone-number countries='["fr", "gb"]' value="+33647652334"></osds-phone-number>

```html
<osds-phone-number countries="['fr', 'gb']" value="+33647652334"></osds-phone-number>
<osds-phone-number countries='["fr", "gb"]' value="+33647652334"></osds-phone-number>

```
2 changes: 1 addition & 1 deletion packages/components/phone-number/src/global.dev.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ odsSetup();

const logger = new OdsLogger('global-dev');
logger.log('init');
Ods.instance().assetPath('../../../packages/tools/stories/public/flags/');
Ods.instance().assetPath('../flags/flags-4x3/');

(window as any).globalMethod = async function () {
logger.log('globalMethod');
Expand Down
9 changes: 1 addition & 8 deletions packages/components/phone-number/src/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
<osds-phone-number clearable value="+33 6426658925"></osds-phone-number>
<br/>
<br/>
<osds-phone-number id="phoneNumberWithSetCountries" clearable value="+33 6426658925"></osds-phone-number>
<osds-phone-number countries='["fr", "gb"]' clearable value="+33 6426658925"></osds-phone-number>

<p>Error</p>
<osds-phone-number countries="all" error value="+33 6426658925"></osds-phone-number>
Expand All @@ -31,12 +31,5 @@
<br/>
<br/>
<osds-phone-number disabled value="+33 6426658925"></osds-phone-number>


<script>
const phoneNumberWithSetCountries = document.getElementById('phoneNumberWithSetCountries');
phoneNumberWithSetCountries.countries = ['fr', 'gb'];
phoneNumberWithSetCountries.addEventListener('odsValueChange', () => console.log('odsValueChange'))
</script>
</body>
</html>
24 changes: 23 additions & 1 deletion packages/components/phone-number/stencil.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import * as jestConfig from './jest.config';

const args = process.argv.slice(2);

export const config: Config = getStencilConfig({
const config: Config = getStencilConfig({
namespace: 'osds-phone-number',
args,
jestConfig: jestConfig.default,
Expand All @@ -26,3 +26,25 @@ export const config: Config = getStencilConfig({
globalScript: 'src/global.test.ts'
},
});


config.outputTargets?.forEach(output => {
if (output.type === 'dist-custom-elements' || output.type === 'www') {
output.copy = output.copy || [];
output.copy.push(
{
src: '../../../../node_modules/flag-icons/flags/4x3/',
dest: `${output.type === 'dist-custom-elements' ? 'dist/' : ''}flags/flags-4x3/`,
}
);
}
if (output.type === 'www') {
output.copy = output.copy || [];
output.copy.push({
src: '../../../../node_modules/flag-icons/flags/4x3/',
dest: 'flags-custom-path/'
});
}
});

export { config };

0 comments on commit 4a0c71d

Please sign in to comment.