From c137d1f37a02f27e2fd46237f0c8b3b40ab77404 Mon Sep 17 00:00:00 2001 From: JC Franco Date: Fri, 27 Sep 2024 10:16:42 -0700 Subject: [PATCH] fix(input-time-zone): fix region mode quirks after update (#10413) **Related Issue:** #10289 ## Summary This addresses some issues that came up from #10345. * use combobox-item metadata for filtering * fix Ho Chi Minh City label * fix selection bug that would cause rendering quirks * sort GMT locales numerically * sort time zone group items ### Notes * there's a small delay after selection for the country label to be rendered, I'll create a follow-up PR to improve this (#10310 might help with this) * we could also explore an option in combobox to make it easier to update the selected item's label between open/close states. cc @ashetland --------- Co-authored-by: Ben Elan --- .../assets/input-time-zone/t9n/messages.json | 2 +- .../input-time-zone/t9n/messages_en.json | 2 +- .../input-time-zone/input-time-zone.e2e.ts | 3 +- .../input-time-zone/input-time-zone.tsx | 39 +++++++--------- .../input-time-zone/interfaces.d.ts | 6 +-- .../src/components/input-time-zone/utils.ts | 44 ++++++++++++++----- 6 files changed, 58 insertions(+), 38 deletions(-) diff --git a/packages/calcite-components/src/components/input-time-zone/assets/input-time-zone/t9n/messages.json b/packages/calcite-components/src/components/input-time-zone/assets/input-time-zone/t9n/messages.json index 34e09cd6deb..57d539d8575 100644 --- a/packages/calcite-components/src/components/input-time-zone/assets/input-time-zone/t9n/messages.json +++ b/packages/calcite-components/src/components/input-time-zone/assets/input-time-zone/t9n/messages.json @@ -255,7 +255,7 @@ "Asia/Famagusta": "Famagusta", "Asia/Gaza": "Gaza", "Asia/Hebron": "Hebron", - "Asia/Ho_Chi_Minh": "Ho Chi Minh", + "Asia/Ho_Chi_Minh": "Ho Chi Minh City", "Asia/Hong_Kong": "Hong Kong", "Asia/Hovd": "Hovd", "Asia/Irkutsk": "Irkutsk", diff --git a/packages/calcite-components/src/components/input-time-zone/assets/input-time-zone/t9n/messages_en.json b/packages/calcite-components/src/components/input-time-zone/assets/input-time-zone/t9n/messages_en.json index 34e09cd6deb..57d539d8575 100644 --- a/packages/calcite-components/src/components/input-time-zone/assets/input-time-zone/t9n/messages_en.json +++ b/packages/calcite-components/src/components/input-time-zone/assets/input-time-zone/t9n/messages_en.json @@ -255,7 +255,7 @@ "Asia/Famagusta": "Famagusta", "Asia/Gaza": "Gaza", "Asia/Hebron": "Hebron", - "Asia/Ho_Chi_Minh": "Ho Chi Minh", + "Asia/Ho_Chi_Minh": "Ho Chi Minh City", "Asia/Hong_Kong": "Hong Kong", "Asia/Hovd": "Hovd", "Asia/Irkutsk": "Irkutsk", diff --git a/packages/calcite-components/src/components/input-time-zone/input-time-zone.e2e.ts b/packages/calcite-components/src/components/input-time-zone/input-time-zone.e2e.ts index 0bcfd22545e..41b46006fe6 100644 --- a/packages/calcite-components/src/components/input-time-zone/input-time-zone.e2e.ts +++ b/packages/calcite-components/src/components/input-time-zone/input-time-zone.e2e.ts @@ -551,10 +551,11 @@ describe("calcite-input-time-zone", () => { await page.waitForTimeout(DEBOUNCE.filter); const selectedTimeZoneItem = await page.find("calcite-input-time-zone >>> calcite-combobox-item[selected]"); + const itemMetadata = await selectedTimeZoneItem.getProperty("metadata"); const expectedTimeZoneItem = testTimeZoneItems[3]; expect(await input.getProperty("value")).toBe(`${expectedTimeZoneItem.offset}`); - expect(await selectedTimeZoneItem.getProperty("value")).toMatch(expectedTimeZoneItem.name); + expect(itemMetadata.filterValue).toContain(expectedTimeZoneItem.name); }); }); }); diff --git a/packages/calcite-components/src/components/input-time-zone/input-time-zone.tsx b/packages/calcite-components/src/components/input-time-zone/input-time-zone.tsx index f0ff733dcb6..73f8e61b552 100644 --- a/packages/calcite-components/src/components/input-time-zone/input-time-zone.tsx +++ b/packages/calcite-components/src/components/input-time-zone/input-time-zone.tsx @@ -267,9 +267,6 @@ export class InputTimeZone } this.selectedTimeZoneItem = timeZoneItem; - requestAnimationFrame(() => { - this.overrideSelectedLabelForRegion(this.open); - }); } /** @@ -370,19 +367,16 @@ export class InputTimeZone * @private */ private overrideSelectedLabelForRegion(open: boolean): void { - if (this.mode !== "region" || !this.selectedTimeZoneItem || !this.comboboxEl?.selectedItems) { + if (this.mode !== "region" || !this.selectedTimeZoneItem) { return; } const { label, metadata } = this.selectedTimeZoneItem; - requestAnimationFrame(() => { - const itemLabel = - !metadata.country || open - ? label - : getSelectedRegionTimeZoneLabel(label, metadata.country, this.messages); - this.comboboxEl.selectedItems[0].textLabel = itemLabel; - }); + this.comboboxEl.selectedItems[0].textLabel = + !metadata.country || open + ? label + : getSelectedRegionTimeZoneLabel(label, metadata.country, this.messages); } private onComboboxBeforeClose = (event: CustomEvent): void => { @@ -409,7 +403,7 @@ export class InputTimeZone return; } - const selected = this.findTimeZoneItemByLabel(selectedItem.textLabel); + const selected = this.findTimeZoneItemByLabel(selectedItem.getAttribute("data-label")); const selectedValue = `${selected.value}`; if (this.value === selectedValue && selected.label === this.selectedTimeZoneItem.label) { @@ -519,12 +513,12 @@ export class InputTimeZone componentDidLoad(): void { setComponentLoaded(this); - this.overrideSelectedLabelForRegion(this.open); this.openChanged(); } componentDidRender(): void { updateHostInteraction(this); + this.overrideSelectedLabelForRegion(this.open); } render(): VNode { @@ -574,15 +568,16 @@ export class InputTimeZone return this.timeZoneItems.map((group) => { const selected = this.selectedTimeZoneItem === group; - const { label, value } = group; + const { label, metadata, value } = group; return ( ); }); @@ -593,20 +588,20 @@ export class InputTimeZone {items.map((item) => { const selected = this.selectedTimeZoneItem === item; - const { label, value } = item; + const { label, metadata, value } = item; return ( - {item.metadata.offset} + {metadata.offset} ); diff --git a/packages/calcite-components/src/components/input-time-zone/interfaces.d.ts b/packages/calcite-components/src/components/input-time-zone/interfaces.d.ts index 2b13fdc0c0a..ecdd6d18d3c 100644 --- a/packages/calcite-components/src/components/input-time-zone/interfaces.d.ts +++ b/packages/calcite-components/src/components/input-time-zone/interfaces.d.ts @@ -22,10 +22,10 @@ export type TimeZoneMode = "offset" | "name" | "region"; export interface TimeZoneItem { label: string; value: T; - filterValue: string | string[]; - metadata?: { - offset?: string; + metadata: { country?: string; + filterValue: string | string[]; + offset?: string; }; } diff --git a/packages/calcite-components/src/components/input-time-zone/utils.ts b/packages/calcite-components/src/components/input-time-zone/utils.ts index da92beb981d..5d3084a7427 100644 --- a/packages/calcite-components/src/components/input-time-zone/utils.ts +++ b/packages/calcite-components/src/components/input-time-zone/utils.ts @@ -68,7 +68,9 @@ export async function createTimeZoneItems( return { label, value, - filterValue: timeZone, + metadata: { + filterValue: timeZone, + }, }; }) .filter((group) => !!group) @@ -93,29 +95,49 @@ export async function createTimeZoneItems( return groups .map(({ label: region, tzs }) => { + tzs.sort((timeZoneA, timeZoneB) => { + const labeledTimeZoneA = getTimeZoneLabel(timeZoneA, messages); + const labeledTimeZoneB = getTimeZoneLabel(timeZoneB, messages); + const gmtTimeZoneString = "Etc/GMT"; + + if (timeZoneA.startsWith(gmtTimeZoneString) && timeZoneB.startsWith(gmtTimeZoneString)) { + // we use the IANA timezone for simpler and consistent sorting across locales + const offsetStringA = timeZoneA.substring(gmtTimeZoneString.length); + const offsetStringB = timeZoneB.substring(gmtTimeZoneString.length); + + const offsetA = offsetStringA === "" ? 0 : parseInt(offsetStringA); + const offsetB = offsetStringB === "" ? 0 : parseInt(offsetStringB); + + return offsetB - offsetA; + } + + return labeledTimeZoneA.localeCompare(labeledTimeZoneB); + }); + return { label: getMessageOrKeyFallback(messages, region), items: tzs.map((timeZone) => { const decimalOffset = timeZoneOffsetToDecimal( getTimeZoneShortOffset(timeZone, effectiveLocale, referenceDateInMs), ); - const filterValue = - toUserFriendlyName(timeZone) + - (region === globalLabel - ? // we add the global label as global group items do not have a unifying region/name - getTimeZoneLabel(globalLabel, messages) - : ""); const label = getTimeZoneLabel(timeZone, messages); + const filterValue = + region === globalLabel + ? // we rely on the label for search since GMT items have their signs inverted (see https://en.wikipedia.org/wiki/Tz_database#Area) + // in addition to the label we also add "Global" and "Etc" to allow searching for these items + `${getTimeZoneLabel(globalLabel, messages)} Etc` + : toUserFriendlyName(timeZone); + const countryCode = getCountry(timeZone); const country = getMessageOrKeyFallback(messages, countryCode); return { label, value: timeZone, - filterValue, metadata: { - offset: decimalOffset, country: country === label ? undefined : country, + filterValue, + offset: decimalOffset, }, }; }), @@ -172,7 +194,9 @@ export async function createTimeZoneItems( return { label, value, - filterValue: tzs.map((tz) => toUserFriendlyName(tz)), + metadata: { + filterValue: tzs.map((tz) => toUserFriendlyName(tz)), + }, }; }) .filter((group) => !!group)