-
Notifications
You must be signed in to change notification settings - Fork 2.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Labs] Timezone Picker #1568
[Labs] Timezone Picker #1568
Changes from 12 commits
356743c
66c6edc
4a4aef2
b6d8302
456303a
7de36a1
67ca676
dad05d1
75dd866
a1ee5cb
5739478
f61607c
185a671
4bb8e12
50f8e78
0b0237c
25176ab
c08169f
3e4a482
8823063
4c2def9
e5dc667
ba10a57
86ade6b
cbf7ee3
72c143a
cd0f2ad
8671cc0
d0f1e4f
f6f8b93
c7b76d2
1946977
b359c1a
020d02f
0cc560c
44e70d1
4d55db2
f237bdc
1054cea
f6858a7
9400904
d2f7f78
e10b3d3
2114c6a
d9d00bc
f652ffe
dc85ca6
b586639
add9e49
900c07e
bbf0ff1
7fc4720
8782648
15947e6
5afc0d7
ef932e9
cd20736
b23c37c
4c8cb85
4ca31e4
7b96c8a
bf15ba5
11631b8
76cad64
bd9ef19
b2b4788
ff49db9
d427502
ab8d5d5
b8e6c32
68ce37c
7f5df58
b0be6b6
ffbb9d3
956b612
76fccc2
c208968
56bf58e
d5ab205
3453ce0
b14a7c2
248f670
2b37613
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -121,15 +121,20 @@ export class Select<T> extends React.Component<ISelectProps<T>, ISelectState<T>> | |
return Select as new () => Select<T>; | ||
} | ||
|
||
public state: ISelectState<T> = { isOpen: false, query: "" }; | ||
|
||
private TypedQueryList = QueryList.ofType<T>(); | ||
private list: QueryList<T>; | ||
private refHandlers = { | ||
queryList: (ref: QueryList<T>) => (this.list = ref), | ||
}; | ||
private previousFocusedElement: HTMLElement; | ||
|
||
constructor(props?: ISelectProps<T>, context?: any) { | ||
super(props, context); | ||
|
||
const query = props && props.inputProps && props.inputProps.value !== undefined ? props.inputProps.value : ""; | ||
this.state = { isOpen: false, query }; | ||
} | ||
|
||
public render() { | ||
// omit props specific to this component, spread the rest. | ||
const { | ||
|
@@ -155,6 +160,13 @@ export class Select<T> extends React.Component<ISelectProps<T>, ISelectState<T>> | |
); | ||
} | ||
|
||
public componentWillReceiveProps(nextProps: ISelectProps<T>) { | ||
const { inputProps: nextInputProps = {} } = nextProps; | ||
if (nextInputProps.value !== undefined && this.state.query !== nextInputProps.value) { | ||
this.setState({ query: nextInputProps.value }); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nice! this seems like a valuable fix and could use a unit test (since in fact you could go so far as to pull this change to a separate PR focused solely on supporting controlled input value. though in truth we probably want an obvious There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Going to start on this in a separate PR There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
} | ||
} | ||
|
||
public componentDidUpdate(_prevProps: ISelectProps<T>, prevState: ISelectState<T>) { | ||
if (this.state.isOpen && !prevState.isOpen && this.list != null) { | ||
this.list.scrollActiveItemIntoView(); | ||
|
@@ -302,12 +314,12 @@ export class Select<T> extends React.Component<ISelectProps<T>, ISelectState<T>> | |
this.setState({ query }); | ||
Utils.safeInvoke(inputProps.onChange, event); | ||
Utils.safeInvoke(onQueryChange, query); | ||
} | ||
}; | ||
|
||
private resetQuery = () => { | ||
const { items, onQueryChange } = this.props; | ||
const query = ""; | ||
this.setState({ activeItem: items[0], query }); | ||
Utils.safeInvoke(onQueryChange, query); | ||
} | ||
}; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,13 +10,13 @@ import { getTimezoneMetadata } from "./timezoneMetadata"; | |
|
||
export type TimezoneDisplayFormat = "offset" | "abbreviation" | "name" | "composite"; | ||
export const TimezoneDisplayFormat = { | ||
/** Abbreviation format i.e. "GMT" */ | ||
/** Abbreviation format i.e. "HST" */ | ||
ABBREVIATION: "abbreviation" as "abbreviation", | ||
/** Composite format i.e. "Pacific/Honolulu (HST) -10:00" */ | ||
COMPOSITE: "composite" as "composite", | ||
/** Name format i.e. "Pacific/Honolulu" */ | ||
NAME: "name" as "name", | ||
/** Offset format i.e. "-07:00" */ | ||
/** Offset format i.e. "-10:00" */ | ||
OFFSET: "offset" as "offset", | ||
}; | ||
|
||
|
@@ -32,7 +32,7 @@ export function formatTimezone( | |
const { abbreviation, offsetAsString } = getTimezoneMetadata(timezone, date); | ||
switch (displayFormat) { | ||
case TimezoneDisplayFormat.ABBREVIATION: | ||
// Fall back to the offset in regions where there is no abbreviation. | ||
// Fall back to the offset when there is no abbreviation. | ||
return abbreviation ? abbreviation : offsetAsString; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. prefer to avoid implicit boolean coercion. |
||
case TimezoneDisplayFormat.NAME: | ||
return timezone; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,24 +10,61 @@ import * as moment from "moment-timezone"; | |
import { getTimezoneMetadata, ITimezoneMetadata } from "./timezoneMetadata"; | ||
import { getLocalTimezone } from "./timezoneUtils"; | ||
|
||
/** Timezone-specific QueryList item */ | ||
export interface ITimezoneItem { | ||
/** Key to be used as the rendered react key. */ | ||
key: string; | ||
/** Text for the timezone. */ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. blank line after each property, before the next doc comment. |
||
text: string; | ||
/** Label for the timezone. */ | ||
label: string; | ||
/** Optional icon for the timezone. */ | ||
iconName?: IconName; | ||
/** The actual timezone. */ | ||
timezone: string; | ||
} | ||
|
||
/** | ||
* 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)); | ||
return moment.tz.names().map(timezone => toTimezoneItem(timezone, date)); | ||
} | ||
|
||
/** | ||
* Get a list of timezone items where there is one timezone per offset | ||
* and optionally the local timezone as the first item. | ||
* The most populous timezone for each offset is chosen. | ||
* @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); | ||
return includeLocalTimezone && local !== undefined | ||
? [local, ...populous] | ||
: populous; | ||
return includeLocalTimezone && local !== undefined ? [local, ...populous] : populous; | ||
} | ||
|
||
/** | ||
* 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 { | ||
const timezone = getLocalTimezone(); | ||
if (timezone !== undefined) { | ||
const timestamp = date.getTime(); | ||
const zonedDate = moment.tz(timestamp, timezone); | ||
const offsetAsString = zonedDate.format("Z"); | ||
return { | ||
iconName: "locate", | ||
key: `${timezone}-local`, | ||
label: offsetAsString, | ||
text: "Current timezone", | ||
timezone, | ||
}; | ||
} else { | ||
return undefined; | ||
} | ||
} | ||
|
||
/** | ||
|
@@ -37,7 +74,7 @@ export function getInitialTimezoneItems(date: Date, includeLocalTimezone: boolea | |
*/ | ||
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 timezones = moment.tz.names().filter(timezone => /\//.test(timezone) && !/Etc\//.test(timezone)); | ||
|
||
const timezoneToMetadata: { [timezone: string]: ITimezoneMetadata } = {}; | ||
for (const timezone of timezones) { | ||
|
@@ -73,24 +110,6 @@ function getPopulousTimezoneItems(date: Date): ITimezoneItem[] { | |
return initialTimezones; | ||
} | ||
|
||
function getLocalTimezoneItem(date: Date): ITimezoneItem | undefined { | ||
const timezone = getLocalTimezone(); | ||
if (timezone !== undefined) { | ||
const timestamp = date.getTime(); | ||
const zonedDate = moment.tz(timestamp, timezone); | ||
const offsetAsString = zonedDate.format("Z"); | ||
return { | ||
iconName: "locate", | ||
key: `${timezone}-local`, | ||
label: offsetAsString, | ||
text: "Current timezone", | ||
timezone, | ||
}; | ||
} else { | ||
return undefined; | ||
} | ||
} | ||
|
||
function toTimezoneItem(timezone: string, date: Date): ITimezoneItem { | ||
const { abbreviation, offsetAsString } = getTimezoneMetadata(timezone, date); | ||
return { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,9 +23,7 @@ export function getTimezoneMetadata(timezone: string, date: Date): ITimezoneMeta | |
const offsetAsString = zonedDate.format("Z"); | ||
const abbreviation = getAbbreviation(timezone, timestamp); | ||
// TODO: amend moment-timezone typings to include the population field | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue to track this? in moment's repo There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
const population = zone && (zone as any).population !== undefined | ||
? (zone as any).population | ||
: undefined; | ||
const population = zone && (zone as any).population !== undefined ? (zone as any).population : undefined; | ||
|
||
return { | ||
timezone, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,14 +21,11 @@ import { | |
MenuItem, | ||
Utils, | ||
} from "@blueprintjs/core"; | ||
import { | ||
ISelectItemRendererProps, | ||
Select, | ||
} from ".."; | ||
import { ISelectItemRendererProps, Select } from ".."; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ❌ import from actual component source file, not the index. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See some other instances of this |
||
import * as Classes from "../../common/classes"; | ||
import { formatTimezone, TimezoneDisplayFormat } from "./timezoneDisplayFormat"; | ||
import { getInitialTimezoneItems, getTimezoneItems, ITimezoneItem } from "./timezoneItems"; | ||
import { createTimezoneQueryEngine, IItemQueryEngine } from "./timezoneQueryEngine"; | ||
import { filterWithQueryCandidates, getTimezoneQueryCandidates } from "./timezoneUtils"; | ||
|
||
export { TimezoneDisplayFormat }; | ||
|
||
|
@@ -118,16 +115,16 @@ export class TimezonePicker extends AbstractComponent<ITimezonePickerProps, ITim | |
}; | ||
|
||
private timezoneItems: ITimezoneItem[]; | ||
private timezoneQueryEngine: IItemQueryEngine<ITimezoneItem>; | ||
private initialTimezoneItems: ITimezoneItem[]; | ||
|
||
constructor(props: ITimezonePickerProps, context?: any) { | ||
super(props, context); | ||
|
||
const { value, date = new Date(), showLocalTimezone } = props; | ||
this.state = { date, value }; | ||
const { value, date = new Date(), showLocalTimezone, inputProps = {} } = props; | ||
const query = inputProps.value !== undefined ? inputProps.value : ""; | ||
this.state = { date, value, query }; | ||
|
||
this.updateTimezones(date); | ||
this.timezoneItems = getTimezoneItems(date); | ||
this.initialTimezoneItems = getInitialTimezoneItems(date, showLocalTimezone); | ||
} | ||
|
||
|
@@ -165,11 +162,11 @@ export class TimezonePicker extends AbstractComponent<ITimezonePickerProps, ITim | |
} | ||
|
||
public componentWillReceiveProps(nextProps: ITimezonePickerProps) { | ||
const { date: nextDate = new Date() } = nextProps; | ||
const { date: nextDate = new Date(), inputProps: nextInputProps = {} } = nextProps; | ||
const dateChanged = this.state.date.getTime() !== nextDate.getTime(); | ||
|
||
if (dateChanged) { | ||
this.updateTimezones(nextDate); | ||
this.timezoneItems = getTimezoneItems(nextDate); | ||
} | ||
if (dateChanged || this.props.showLocalTimezone !== nextProps.showLocalTimezone) { | ||
this.initialTimezoneItems = getInitialTimezoneItems(nextDate, nextProps.showLocalTimezone); | ||
|
@@ -182,6 +179,9 @@ export class TimezonePicker extends AbstractComponent<ITimezonePickerProps, ITim | |
if (this.state.value !== nextProps.value) { | ||
nextState.value = nextProps.value; | ||
} | ||
if (nextInputProps.value !== undefined && this.state.query !== nextInputProps.value) { | ||
nextState.query = nextInputProps.value; | ||
} | ||
this.setState(nextState); | ||
} | ||
|
||
|
@@ -208,15 +208,19 @@ export class TimezonePicker extends AbstractComponent<ITimezonePickerProps, ITim | |
); | ||
} | ||
|
||
private updateTimezones(date: Date): void { | ||
this.timezoneItems = getTimezoneItems(date); | ||
this.timezoneQueryEngine = createTimezoneQueryEngine(date); | ||
} | ||
|
||
private filterItems = (query: string, items: ITimezoneItem[]): ITimezoneItem[] => { | ||
// Only filter and rank the non-initial items | ||
return items === this.initialTimezoneItems ? items : this.timezoneQueryEngine.filterAndRankItems(query, items); | ||
} | ||
if (query === "") { | ||
return items; | ||
} | ||
|
||
const { date } = this.state; | ||
return filterWithQueryCandidates( | ||
items, | ||
query, | ||
item => item.timezone, | ||
item => getTimezoneQueryCandidates(item.timezone, date), | ||
); | ||
}; | ||
|
||
private renderItem = (itemProps: ISelectItemRendererProps<ITimezoneItem>) => { | ||
const { item, isActive, handleClick } = itemProps; | ||
|
@@ -236,16 +240,16 @@ export class TimezonePicker extends AbstractComponent<ITimezonePickerProps, ITim | |
shouldDismissPopover={false} | ||
/> | ||
); | ||
} | ||
}; | ||
|
||
private handleItemSelect = (timezone: ITimezoneItem) => { | ||
if (this.props.value === undefined) { | ||
this.setState({ value: timezone.timezone }); | ||
} | ||
Utils.safeInvoke(this.props.onChange, timezone.timezone); | ||
} | ||
}; | ||
|
||
private handleQueryChange = (query: string) => { | ||
this.setState({ query }); | ||
} | ||
}; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please always wrap params in parens. this should be a lint failure...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, this is the prettier standard