Skip to content

Commit

Permalink
fix(input-time-zone): fix region mode quirks after update (#10413)
Browse files Browse the repository at this point in the history
**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 <no-reply@benelan.dev>
  • Loading branch information
jcfranco and benelan authored Sep 27, 2024
1 parent 699e166 commit c137d1f
Show file tree
Hide file tree
Showing 6 changed files with 58 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -267,9 +267,6 @@ export class InputTimeZone
}

this.selectedTimeZoneItem = timeZoneItem;
requestAnimationFrame(() => {
this.overrideSelectedLabelForRegion(this.open);
});
}

/**
Expand Down Expand Up @@ -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 => {
Expand All @@ -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) {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 (
<calcite-combobox-item
data-value={value}
data-label={label}
key={label}
metadata={metadata}
selected={selected}
textLabel={label}
value={`${group.filterValue}`}
value={value}
/>
);
});
Expand All @@ -593,20 +588,20 @@ export class InputTimeZone
<calcite-combobox-item-group key={label} label={label}>
{items.map((item) => {
const selected = this.selectedTimeZoneItem === item;
const { label, value } = item;
const { label, metadata, value } = item;

return (
<calcite-combobox-item
data-value={value}
description={item.metadata.country}
data-label={label}
description={metadata.country}
key={label}
metadata={item.metadata}
metadata={metadata}
selected={selected}
textLabel={label}
value={`${item.filterValue}`}
value={value}
>
<span class={CSS.offset} slot="content-end">
{item.metadata.offset}
{metadata.offset}
</span>
</calcite-combobox-item>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ export type TimeZoneMode = "offset" | "name" | "region";
export interface TimeZoneItem<T extends number | string = number | string> {
label: string;
value: T;
filterValue: string | string[];
metadata?: {
offset?: string;
metadata: {
country?: string;
filterValue: string | string[];
offset?: string;
};
}

Expand Down
44 changes: 34 additions & 10 deletions packages/calcite-components/src/components/input-time-zone/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,9 @@ export async function createTimeZoneItems(
return {
label,
value,
filterValue: timeZone,
metadata: {
filterValue: timeZone,
},
};
})
.filter((group) => !!group)
Expand All @@ -93,29 +95,49 @@ export async function createTimeZoneItems(

return groups
.map<TimeZoneItemGroup>(({ 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,
},
};
}),
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit c137d1f

Please sign in to comment.