From 43c5c94855ccbb4e35331d5c5e8fdc28f777d1cc Mon Sep 17 00:00:00 2001 From: Phil Chen Date: Fri, 1 Jun 2018 17:29:01 -0400 Subject: [PATCH 1/4] Add useManualCalculation flag for TimezonePicker --- .../timezone-picker/timezoneDisplayFormat.ts | 3 +- .../timezone-picker/timezoneItems.ts | 30 +++++---- .../timezone-picker/timezoneMetadata.ts | 64 +++++++++++++------ .../timezone-picker/timezonePicker.tsx | 29 ++++++--- .../timezone-picker/timezoneUtils.ts | 4 +- packages/labs/test/timezonePickerTests.tsx | 13 ++-- 6 files changed, 96 insertions(+), 47 deletions(-) diff --git a/packages/labs/src/components/timezone-picker/timezoneDisplayFormat.ts b/packages/labs/src/components/timezone-picker/timezoneDisplayFormat.ts index d0ddd5874d..810bbe3a10 100644 --- a/packages/labs/src/components/timezone-picker/timezoneDisplayFormat.ts +++ b/packages/labs/src/components/timezone-picker/timezoneDisplayFormat.ts @@ -23,12 +23,13 @@ export function formatTimezone( timezone: string | undefined, date: Date, displayFormat: TimezoneDisplayFormat, + useManualCalc: boolean, ): string | undefined { if (!timezone || !moment.tz.zone(timezone)) { return undefined; } - const { abbreviation, offsetAsString } = getTimezoneMetadata(timezone, date); + const { abbreviation, offsetAsString } = getTimezoneMetadata(timezone, date, useManualCalc); switch (displayFormat) { case TimezoneDisplayFormat.ABBREVIATION: // Fall back to the offset when there is no abbreviation. diff --git a/packages/labs/src/components/timezone-picker/timezoneItems.ts b/packages/labs/src/components/timezone-picker/timezoneItems.ts index c5f4ecab9a..15cd3ea7d8 100644 --- a/packages/labs/src/components/timezone-picker/timezoneItems.ts +++ b/packages/labs/src/components/timezone-picker/timezoneItems.ts @@ -31,8 +31,8 @@ export interface ITimezoneItem { * Get a list of all timezone items. * @param date the date to use when determining timezone offsets */ -export function getTimezoneItems(date: Date): ITimezoneItem[] { - return moment.tz.names().map(timezone => toTimezoneItem(timezone, date)); +export function getTimezoneItems(date: Date, useManualCalc: boolean): ITimezoneItem[] { + return moment.tz.names().map(timezone => toTimezoneItem(timezone, date, useManualCalc)); } /** @@ -42,9 +42,13 @@ export function getTimezoneItems(date: Date): ITimezoneItem[] { * @param date the date to use when determining timezone offsets * @param includeLocalTimezone whether to include the local timezone */ -export function getInitialTimezoneItems(date: Date, includeLocalTimezone: boolean): ITimezoneItem[] { - const populous = getPopulousTimezoneItems(date); - const local = getLocalTimezoneItem(date); +export function getInitialTimezoneItems( + date: Date, + includeLocalTimezone: boolean, + useManualCalc: boolean, +): ITimezoneItem[] { + const populous = getPopulousTimezoneItems(date, useManualCalc); + const local = getLocalTimezoneItem(date, useManualCalc); return includeLocalTimezone && local !== undefined ? [local, ...populous] : populous; } @@ -52,12 +56,10 @@ export function getInitialTimezoneItems(date: Date, includeLocalTimezone: boolea * Get the timezone item for the user's local timezone. * @param date the date to use when determining timezone offsets */ -export function getLocalTimezoneItem(date: Date): ITimezoneItem | undefined { +export function getLocalTimezoneItem(date: Date, useManualCalc: boolean): ITimezoneItem | undefined { const timezone = getLocalTimezone(); if (timezone !== undefined) { - const timestamp = date.getTime(); - const zonedDate = moment.tz(timestamp, timezone); - const offsetAsString = zonedDate.format("Z"); + const { offsetAsString } = getTimezoneMetadata(timezone, date, useManualCalc); return { iconName: "locate", key: `${timezone}-local`, @@ -75,13 +77,13 @@ export function getLocalTimezoneItem(date: Date): ITimezoneItem | undefined { * than one region for the offset. * @param date the date to use when determining timezone offsets */ -function getPopulousTimezoneItems(date: Date): ITimezoneItem[] { +function getPopulousTimezoneItems(date: Date, useManualCalc: boolean): ITimezoneItem[] { // Filter out noisy timezones. See https://github.com/moment/moment-timezone/issues/227 const timezones = moment.tz.names().filter(timezone => /\//.test(timezone) && !/Etc\//.test(timezone)); const timezoneToMetadata: { [timezone: string]: ITimezoneMetadata } = {}; for (const timezone of timezones) { - timezoneToMetadata[timezone] = getTimezoneMetadata(timezone, date); + timezoneToMetadata[timezone] = getTimezoneMetadata(timezone, date, useManualCalc); } // Order by offset ascending, population descending, timezone name ascending @@ -106,15 +108,15 @@ function getPopulousTimezoneItems(date: Date): ITimezoneItem[] { for (const timezone of timezones) { const curOffset = timezoneToMetadata[timezone].offset; if (prevOffset === undefined || prevOffset !== curOffset) { - initialTimezones.push(toTimezoneItem(timezone, date)); + initialTimezones.push(toTimezoneItem(timezone, date, useManualCalc)); prevOffset = curOffset; } } return initialTimezones; } -function toTimezoneItem(timezone: string, date: Date): ITimezoneItem { - const { abbreviation, offsetAsString } = getTimezoneMetadata(timezone, date); +function toTimezoneItem(timezone: string, date: Date, useManualCalc: boolean): ITimezoneItem { + const { abbreviation, offsetAsString } = getTimezoneMetadata(timezone, date, useManualCalc); return { key: timezone, label: offsetAsString, diff --git a/packages/labs/src/components/timezone-picker/timezoneMetadata.ts b/packages/labs/src/components/timezone-picker/timezoneMetadata.ts index 07603706a2..487431a374 100644 --- a/packages/labs/src/components/timezone-picker/timezoneMetadata.ts +++ b/packages/labs/src/components/timezone-picker/timezoneMetadata.ts @@ -14,14 +14,26 @@ export interface ITimezoneMetadata { population: number | undefined; } -export function getTimezoneMetadata(timezone: string, date: Date): ITimezoneMetadata { - const timestamp = date.getTime(); +export function getTimezoneMetadata(timezone: string, date: Date, useManualCalc: boolean): ITimezoneMetadata { const zone = moment.tz.zone(timezone); + const timestamp = date.getTime(); + const calculateMetadata = useManualCalc ? getMetadataFromMomentManual : getMetadataFromMoment; + return calculateMetadata(timezone, zone, timestamp); +} + +function getValidAbbreviation(abbreviation: string) { + return isValidAbbreviation(abbreviation) ? abbreviation : undefined; +} + +function isValidAbbreviation(abbreviation: string) { + return abbreviation != null && abbreviation.length > 0 && abbreviation[0] !== "-" && abbreviation[0] !== "+"; +} + +function getMetadataFromMoment(timezone: string, zone: moment.MomentZone, timestamp: number) { const zonedDate = moment.tz(timestamp, timezone); const offset = zonedDate.utcOffset(); const offsetAsString = zonedDate.format("Z"); - const abbreviation = getAbbreviation(timezone, timestamp); - + const abbreviation = getValidAbbreviation(zonedDate.zoneAbbr()); return { abbreviation, offset, @@ -31,22 +43,38 @@ export function getTimezoneMetadata(timezone: string, date: Date): ITimezoneMeta }; } -/** - * Get the abbreviation for a timezone. - * We need this utility because moment-timezone's `abbr` will not always give the abbreviated time zone name, - * since it falls back to the time offsets for each region. - * https://momentjs.com/timezone/docs/#/using-timezones/formatting/ - */ -function getAbbreviation(timezone: string, timestamp: number): string | undefined { - const zone = moment.tz.zone(timezone); - if (zone) { - const abbreviation = zone.abbr(timestamp); +function getMetadataFromMomentManual(timezone: string, zone: moment.MomentZone, timestamp: number) { + const { abbrs, offsets, population, untils } = zone; + const index = findOffsetIndex(timestamp, untils); + const abbreviation = getValidAbbreviation(abbrs[index]); + const offset = offsets[index] * -1; + const offsetAsString = getOffsetAsString(offset); + return { + abbreviation, + offset, + offsetAsString, + population, + timezone, + }; +} - // Only include abbreviations that are not just a repeat of the offset - if (abbreviation.length > 0 && abbreviation[0] !== "-" && abbreviation[0] !== "+") { - return abbreviation; +function findOffsetIndex(timestamp: number, untils: number[]) { + for (let i = 0; i < untils.length; i++) { + if (i === untils.length - 1 || timestamp < untils[i]) { + return i; } } + return 0; +} + +function getOffsetAsString(offset: number) { + const offsetVal = Math.abs(offset); + const minutes = offsetVal % 60; + const hours = (offsetVal - minutes) / 60; + const sign = offset >= 0 ? "+" : "-"; + return sign + lpad(hours) + ":" + lpad(minutes); +} - return undefined; +function lpad(num: number) { + return num < 10 ? "0" + num : num; } diff --git a/packages/labs/src/components/timezone-picker/timezonePicker.tsx b/packages/labs/src/components/timezone-picker/timezonePicker.tsx index 293ca3ced3..58d9af42db 100644 --- a/packages/labs/src/components/timezone-picker/timezonePicker.tsx +++ b/packages/labs/src/components/timezone-picker/timezonePicker.tsx @@ -60,6 +60,12 @@ export interface ITimezonePickerProps extends IProps { */ showLocalTimezone?: boolean; + /** + * Whether to use manual calculation (faster, but possibly less accurate) rather than moment-timezone to infer metadata; + * @default false + */ + useManualCalc?: boolean; + /** * Format to use when displaying the selected (or default) timezone within the target element. * @default TimezoneDisplayFormat.OFFSET @@ -111,6 +117,7 @@ export class TimezonePicker extends AbstractComponent getTimezoneQueryCandidates(item.timezone, date)); + return filterWithQueryCandidates(items, query, item => + getTimezoneQueryCandidates(item.timezone, date, useManualCalc), + ); }; private renderItem = (itemProps: ISelectItemRendererProps) => { diff --git a/packages/labs/src/components/timezone-picker/timezoneUtils.ts b/packages/labs/src/components/timezone-picker/timezoneUtils.ts index 5406ecf0c8..93ceb4d781 100644 --- a/packages/labs/src/components/timezone-picker/timezoneUtils.ts +++ b/packages/labs/src/components/timezone-picker/timezoneUtils.ts @@ -26,8 +26,8 @@ export function getLocalTimezone(): string | undefined { * @param date the date to use when determining timezone offsets * @returns a list of queryable strings */ -export function getTimezoneQueryCandidates(timezone: string, date: Date): string[] { - const { abbreviation, offsetAsString } = getTimezoneMetadata(timezone, date); +export function getTimezoneQueryCandidates(timezone: string, date: Date, useManualCalc: boolean): string[] { + const { abbreviation, offsetAsString } = getTimezoneMetadata(timezone, date, useManualCalc); return [timezone, abbreviation, offsetAsString].filter(candidate => candidate !== undefined); } diff --git a/packages/labs/test/timezonePickerTests.tsx b/packages/labs/test/timezonePickerTests.tsx index 01c589de1e..1274eb5df8 100644 --- a/packages/labs/test/timezonePickerTests.tsx +++ b/packages/labs/test/timezonePickerTests.tsx @@ -74,7 +74,7 @@ describe("", () => { const date = new Date(); const timezonePicker = shallow(); const items = findSelect(timezonePicker).prop("items"); - assert.deepEqual(items, getInitialTimezoneItems(date, true)); + assert.deepEqual(items, getInitialTimezoneItems(date, true, false)); }); it("if inputProps.value is non-empty, all items are shown", () => { @@ -85,7 +85,7 @@ describe("", () => { ); assert.strictEqual(timezonePicker.state("query"), query); const items = findSelect(timezonePicker).prop("items"); - assert.deepEqual(items, getTimezoneItems(date)); + assert.deepEqual(items, getTimezoneItems(date, false)); }); it("if inputProps.value is non-empty and it changes to a different non-empty value, the same items are shown", () => { @@ -129,7 +129,7 @@ describe("", () => { const items = findSelect(timezonePicker).prop("items"); assert.isTrue(items.length > 0); const firstItem = items[0]; - const expectedFirstItem = getInitialTimezoneItems(date, false)[0]; + const expectedFirstItem = getInitialTimezoneItems(date, false, false)[0]; assert.deepEqual(firstItem, expectedFirstItem); }); @@ -140,7 +140,7 @@ describe("", () => { , ); const items = findSelect(timezonePicker).prop("items"); - const localTimezoneItem = getLocalTimezoneItem(date); + const localTimezoneItem = getLocalTimezoneItem(date, false); const itemsWithLocalTimezone = items.filter(item => item.timezone === localTimezoneItem.timezone); for (const item of itemsWithLocalTimezone) { assert.notDeepEqual(item, localTimezoneItem); @@ -209,6 +209,11 @@ describe("", () => { }); }); + it("manual metadata matches moment metadata", () => { + const date = new Date(); + assert.deepEqual(getTimezoneItems(date, true), getTimezoneItems(date, false)); + }); + it("invokes the onChange input prop", () => { const query = "test query"; const onInputChange = sinon.spy(); From 511c08745702f00e90293a31267537a302afde38 Mon Sep 17 00:00:00 2001 From: Phil Chen Date: Fri, 1 Jun 2018 17:34:58 -0400 Subject: [PATCH 2/4] Comments --- .../timezone-picker/timezoneMetadata.ts | 21 ++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/packages/labs/src/components/timezone-picker/timezoneMetadata.ts b/packages/labs/src/components/timezone-picker/timezoneMetadata.ts index 487431a374..4ebd245095 100644 --- a/packages/labs/src/components/timezone-picker/timezoneMetadata.ts +++ b/packages/labs/src/components/timezone-picker/timezoneMetadata.ts @@ -21,19 +21,26 @@ export function getTimezoneMetadata(timezone: string, date: Date, useManualCalc: return calculateMetadata(timezone, zone, timestamp); } -function getValidAbbreviation(abbreviation: string) { - return isValidAbbreviation(abbreviation) ? abbreviation : undefined; +/** + * Ignore abbreviations that are simply offsets, i.e. "+14" instead of "PST" + * @param abbreviation + */ +function getNonOffsetAbbreviation(abbreviation: string) { + return isNonOffsetAbbreviation(abbreviation) ? abbreviation : undefined; } -function isValidAbbreviation(abbreviation: string) { +function isNonOffsetAbbreviation(abbreviation: string) { return abbreviation != null && abbreviation.length > 0 && abbreviation[0] !== "-" && abbreviation[0] !== "+"; } +/** + * Use moment-timezone to parse the timestamp and provide timezone metadata + */ function getMetadataFromMoment(timezone: string, zone: moment.MomentZone, timestamp: number) { const zonedDate = moment.tz(timestamp, timezone); const offset = zonedDate.utcOffset(); const offsetAsString = zonedDate.format("Z"); - const abbreviation = getValidAbbreviation(zonedDate.zoneAbbr()); + const abbreviation = getNonOffsetAbbreviation(zonedDate.zoneAbbr()); return { abbreviation, offset, @@ -43,10 +50,14 @@ function getMetadataFromMoment(timezone: string, zone: moment.MomentZone, timest }; } +/** + * Manually determine timezone metadata by skipping the timestamp parsing and following + * http://momentjs.com/timezone/docs/#/data-formats/unpacked-format/ + */ function getMetadataFromMomentManual(timezone: string, zone: moment.MomentZone, timestamp: number) { const { abbrs, offsets, population, untils } = zone; const index = findOffsetIndex(timestamp, untils); - const abbreviation = getValidAbbreviation(abbrs[index]); + const abbreviation = getNonOffsetAbbreviation(abbrs[index]); const offset = offsets[index] * -1; const offsetAsString = getOffsetAsString(offset); return { From c491a6ec18762eae9d2b24d6e276115c05fa65fd Mon Sep 17 00:00:00 2001 From: Phil Chen Date: Fri, 8 Jun 2018 15:04:27 -0400 Subject: [PATCH 3/4] Remove useManualCalc fork --- .../timezone-picker/timezoneDisplayFormat.ts | 3 +- .../timezone-picker/timezoneItems.ts | 28 +++++----- .../timezone-picker/timezoneMetadata.ts | 52 +++++-------------- .../timezone-picker/timezonePicker.tsx | 29 +++-------- .../timezone-picker/timezoneUtils.ts | 4 +- packages/labs/test/timezonePickerTests.tsx | 13 ++--- 6 files changed, 40 insertions(+), 89 deletions(-) diff --git a/packages/labs/src/components/timezone-picker/timezoneDisplayFormat.ts b/packages/labs/src/components/timezone-picker/timezoneDisplayFormat.ts index 810bbe3a10..d0ddd5874d 100644 --- a/packages/labs/src/components/timezone-picker/timezoneDisplayFormat.ts +++ b/packages/labs/src/components/timezone-picker/timezoneDisplayFormat.ts @@ -23,13 +23,12 @@ export function formatTimezone( timezone: string | undefined, date: Date, displayFormat: TimezoneDisplayFormat, - useManualCalc: boolean, ): string | undefined { if (!timezone || !moment.tz.zone(timezone)) { return undefined; } - const { abbreviation, offsetAsString } = getTimezoneMetadata(timezone, date, useManualCalc); + const { abbreviation, offsetAsString } = getTimezoneMetadata(timezone, date); switch (displayFormat) { case TimezoneDisplayFormat.ABBREVIATION: // Fall back to the offset when there is no abbreviation. diff --git a/packages/labs/src/components/timezone-picker/timezoneItems.ts b/packages/labs/src/components/timezone-picker/timezoneItems.ts index 15cd3ea7d8..ea1eb21240 100644 --- a/packages/labs/src/components/timezone-picker/timezoneItems.ts +++ b/packages/labs/src/components/timezone-picker/timezoneItems.ts @@ -31,8 +31,8 @@ export interface ITimezoneItem { * Get a list of all timezone items. * @param date the date to use when determining timezone offsets */ -export function getTimezoneItems(date: Date, useManualCalc: boolean): ITimezoneItem[] { - return moment.tz.names().map(timezone => toTimezoneItem(timezone, date, useManualCalc)); +export function getTimezoneItems(date: Date): ITimezoneItem[] { + return moment.tz.names().map(timezone => toTimezoneItem(timezone, date)); } /** @@ -42,13 +42,9 @@ export function getTimezoneItems(date: Date, useManualCalc: boolean): ITimezoneI * @param date the date to use when determining timezone offsets * @param includeLocalTimezone whether to include the local timezone */ -export function getInitialTimezoneItems( - date: Date, - includeLocalTimezone: boolean, - useManualCalc: boolean, -): ITimezoneItem[] { - const populous = getPopulousTimezoneItems(date, useManualCalc); - const local = getLocalTimezoneItem(date, useManualCalc); +export function getInitialTimezoneItems(date: Date, includeLocalTimezone: boolean): ITimezoneItem[] { + const populous = getPopulousTimezoneItems(date); + const local = getLocalTimezoneItem(date); return includeLocalTimezone && local !== undefined ? [local, ...populous] : populous; } @@ -56,10 +52,10 @@ export function getInitialTimezoneItems( * Get the timezone item for the user's local timezone. * @param date the date to use when determining timezone offsets */ -export function getLocalTimezoneItem(date: Date, useManualCalc: boolean): ITimezoneItem | undefined { +export function getLocalTimezoneItem(date: Date): ITimezoneItem | undefined { const timezone = getLocalTimezone(); if (timezone !== undefined) { - const { offsetAsString } = getTimezoneMetadata(timezone, date, useManualCalc); + const { offsetAsString } = getTimezoneMetadata(timezone, date); return { iconName: "locate", key: `${timezone}-local`, @@ -77,13 +73,13 @@ export function getLocalTimezoneItem(date: Date, useManualCalc: boolean): ITimez * than one region for the offset. * @param date the date to use when determining timezone offsets */ -function getPopulousTimezoneItems(date: Date, useManualCalc: boolean): ITimezoneItem[] { +function getPopulousTimezoneItems(date: Date): ITimezoneItem[] { // Filter out noisy timezones. See https://github.com/moment/moment-timezone/issues/227 const timezones = moment.tz.names().filter(timezone => /\//.test(timezone) && !/Etc\//.test(timezone)); const timezoneToMetadata: { [timezone: string]: ITimezoneMetadata } = {}; for (const timezone of timezones) { - timezoneToMetadata[timezone] = getTimezoneMetadata(timezone, date, useManualCalc); + timezoneToMetadata[timezone] = getTimezoneMetadata(timezone, date); } // Order by offset ascending, population descending, timezone name ascending @@ -108,15 +104,15 @@ function getPopulousTimezoneItems(date: Date, useManualCalc: boolean): ITimezone for (const timezone of timezones) { const curOffset = timezoneToMetadata[timezone].offset; if (prevOffset === undefined || prevOffset !== curOffset) { - initialTimezones.push(toTimezoneItem(timezone, date, useManualCalc)); + initialTimezones.push(toTimezoneItem(timezone, date)); prevOffset = curOffset; } } return initialTimezones; } -function toTimezoneItem(timezone: string, date: Date, useManualCalc: boolean): ITimezoneItem { - const { abbreviation, offsetAsString } = getTimezoneMetadata(timezone, date, useManualCalc); +function toTimezoneItem(timezone: string, date: Date): ITimezoneItem { + const { abbreviation, offsetAsString } = getTimezoneMetadata(timezone, date); return { key: timezone, label: offsetAsString, diff --git a/packages/labs/src/components/timezone-picker/timezoneMetadata.ts b/packages/labs/src/components/timezone-picker/timezoneMetadata.ts index 4ebd245095..988207ab79 100644 --- a/packages/labs/src/components/timezone-picker/timezoneMetadata.ts +++ b/packages/labs/src/components/timezone-picker/timezoneMetadata.ts @@ -14,47 +14,9 @@ export interface ITimezoneMetadata { population: number | undefined; } -export function getTimezoneMetadata(timezone: string, date: Date, useManualCalc: boolean): ITimezoneMetadata { +export function getTimezoneMetadata(timezone: string, date: Date): ITimezoneMetadata { const zone = moment.tz.zone(timezone); const timestamp = date.getTime(); - const calculateMetadata = useManualCalc ? getMetadataFromMomentManual : getMetadataFromMoment; - return calculateMetadata(timezone, zone, timestamp); -} - -/** - * Ignore abbreviations that are simply offsets, i.e. "+14" instead of "PST" - * @param abbreviation - */ -function getNonOffsetAbbreviation(abbreviation: string) { - return isNonOffsetAbbreviation(abbreviation) ? abbreviation : undefined; -} - -function isNonOffsetAbbreviation(abbreviation: string) { - return abbreviation != null && abbreviation.length > 0 && abbreviation[0] !== "-" && abbreviation[0] !== "+"; -} - -/** - * Use moment-timezone to parse the timestamp and provide timezone metadata - */ -function getMetadataFromMoment(timezone: string, zone: moment.MomentZone, timestamp: number) { - const zonedDate = moment.tz(timestamp, timezone); - const offset = zonedDate.utcOffset(); - const offsetAsString = zonedDate.format("Z"); - const abbreviation = getNonOffsetAbbreviation(zonedDate.zoneAbbr()); - return { - abbreviation, - offset, - offsetAsString, - population: zone.population, - timezone, - }; -} - -/** - * Manually determine timezone metadata by skipping the timestamp parsing and following - * http://momentjs.com/timezone/docs/#/data-formats/unpacked-format/ - */ -function getMetadataFromMomentManual(timezone: string, zone: moment.MomentZone, timestamp: number) { const { abbrs, offsets, population, untils } = zone; const index = findOffsetIndex(timestamp, untils); const abbreviation = getNonOffsetAbbreviation(abbrs[index]); @@ -69,6 +31,18 @@ function getMetadataFromMomentManual(timezone: string, zone: moment.MomentZone, }; } +/** + * Ignore abbreviations that are simply offsets, i.e. "+14" instead of "PST" + * @param abbreviation + */ +function getNonOffsetAbbreviation(abbreviation: string) { + return isNonOffsetAbbreviation(abbreviation) ? abbreviation : undefined; +} + +function isNonOffsetAbbreviation(abbreviation: string) { + return abbreviation != null && abbreviation.length > 0 && abbreviation[0] !== "-" && abbreviation[0] !== "+"; +} + function findOffsetIndex(timestamp: number, untils: number[]) { for (let i = 0; i < untils.length; i++) { if (i === untils.length - 1 || timestamp < untils[i]) { diff --git a/packages/labs/src/components/timezone-picker/timezonePicker.tsx b/packages/labs/src/components/timezone-picker/timezonePicker.tsx index 58d9af42db..293ca3ced3 100644 --- a/packages/labs/src/components/timezone-picker/timezonePicker.tsx +++ b/packages/labs/src/components/timezone-picker/timezonePicker.tsx @@ -60,12 +60,6 @@ export interface ITimezonePickerProps extends IProps { */ showLocalTimezone?: boolean; - /** - * Whether to use manual calculation (faster, but possibly less accurate) rather than moment-timezone to infer metadata; - * @default false - */ - useManualCalc?: boolean; - /** * Format to use when displaying the selected (or default) timezone within the target element. * @default TimezoneDisplayFormat.OFFSET @@ -117,7 +111,6 @@ export class TimezonePicker extends AbstractComponent - getTimezoneQueryCandidates(item.timezone, date, useManualCalc), - ); + return filterWithQueryCandidates(items, query, item => getTimezoneQueryCandidates(item.timezone, date)); }; private renderItem = (itemProps: ISelectItemRendererProps) => { diff --git a/packages/labs/src/components/timezone-picker/timezoneUtils.ts b/packages/labs/src/components/timezone-picker/timezoneUtils.ts index 93ceb4d781..5406ecf0c8 100644 --- a/packages/labs/src/components/timezone-picker/timezoneUtils.ts +++ b/packages/labs/src/components/timezone-picker/timezoneUtils.ts @@ -26,8 +26,8 @@ export function getLocalTimezone(): string | undefined { * @param date the date to use when determining timezone offsets * @returns a list of queryable strings */ -export function getTimezoneQueryCandidates(timezone: string, date: Date, useManualCalc: boolean): string[] { - const { abbreviation, offsetAsString } = getTimezoneMetadata(timezone, date, useManualCalc); +export function getTimezoneQueryCandidates(timezone: string, date: Date): string[] { + const { abbreviation, offsetAsString } = getTimezoneMetadata(timezone, date); return [timezone, abbreviation, offsetAsString].filter(candidate => candidate !== undefined); } diff --git a/packages/labs/test/timezonePickerTests.tsx b/packages/labs/test/timezonePickerTests.tsx index 1274eb5df8..01c589de1e 100644 --- a/packages/labs/test/timezonePickerTests.tsx +++ b/packages/labs/test/timezonePickerTests.tsx @@ -74,7 +74,7 @@ describe("", () => { const date = new Date(); const timezonePicker = shallow(); const items = findSelect(timezonePicker).prop("items"); - assert.deepEqual(items, getInitialTimezoneItems(date, true, false)); + assert.deepEqual(items, getInitialTimezoneItems(date, true)); }); it("if inputProps.value is non-empty, all items are shown", () => { @@ -85,7 +85,7 @@ describe("", () => { ); assert.strictEqual(timezonePicker.state("query"), query); const items = findSelect(timezonePicker).prop("items"); - assert.deepEqual(items, getTimezoneItems(date, false)); + assert.deepEqual(items, getTimezoneItems(date)); }); it("if inputProps.value is non-empty and it changes to a different non-empty value, the same items are shown", () => { @@ -129,7 +129,7 @@ describe("", () => { const items = findSelect(timezonePicker).prop("items"); assert.isTrue(items.length > 0); const firstItem = items[0]; - const expectedFirstItem = getInitialTimezoneItems(date, false, false)[0]; + const expectedFirstItem = getInitialTimezoneItems(date, false)[0]; assert.deepEqual(firstItem, expectedFirstItem); }); @@ -140,7 +140,7 @@ describe("", () => { , ); const items = findSelect(timezonePicker).prop("items"); - const localTimezoneItem = getLocalTimezoneItem(date, false); + const localTimezoneItem = getLocalTimezoneItem(date); const itemsWithLocalTimezone = items.filter(item => item.timezone === localTimezoneItem.timezone); for (const item of itemsWithLocalTimezone) { assert.notDeepEqual(item, localTimezoneItem); @@ -209,11 +209,6 @@ describe("", () => { }); }); - it("manual metadata matches moment metadata", () => { - const date = new Date(); - assert.deepEqual(getTimezoneItems(date, true), getTimezoneItems(date, false)); - }); - it("invokes the onChange input prop", () => { const query = "test query"; const onInputChange = sinon.spy(); From 443086beb440adca82f23f140a5272aff8dc7351 Mon Sep 17 00:00:00 2001 From: Phil Chen Date: Mon, 11 Jun 2018 15:35:16 -0400 Subject: [PATCH 4/4] Don't use excess variables and template string --- .../components/timezone-picker/timezoneMetadata.ts | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/packages/labs/src/components/timezone-picker/timezoneMetadata.ts b/packages/labs/src/components/timezone-picker/timezoneMetadata.ts index 988207ab79..7785f6e608 100644 --- a/packages/labs/src/components/timezone-picker/timezoneMetadata.ts +++ b/packages/labs/src/components/timezone-picker/timezoneMetadata.ts @@ -15,17 +15,13 @@ export interface ITimezoneMetadata { } export function getTimezoneMetadata(timezone: string, date: Date): ITimezoneMetadata { - const zone = moment.tz.zone(timezone); - const timestamp = date.getTime(); - const { abbrs, offsets, population, untils } = zone; - const index = findOffsetIndex(timestamp, untils); - const abbreviation = getNonOffsetAbbreviation(abbrs[index]); + const { abbrs, offsets, population, untils } = moment.tz.zone(timezone); + const index = findOffsetIndex(date.getTime(), untils); const offset = offsets[index] * -1; - const offsetAsString = getOffsetAsString(offset); return { - abbreviation, + abbreviation: getNonOffsetAbbreviation(abbrs[index]), offset, - offsetAsString, + offsetAsString: getOffsetAsString(offset), population, timezone, }; @@ -57,7 +53,7 @@ function getOffsetAsString(offset: number) { const minutes = offsetVal % 60; const hours = (offsetVal - minutes) / 60; const sign = offset >= 0 ? "+" : "-"; - return sign + lpad(hours) + ":" + lpad(minutes); + return `${sign}${lpad(hours)}:${lpad(minutes)}`; } function lpad(num: number) {