-
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
Enum Stringification #1921
Enum Stringification #1921
Changes from all commits
283a303
8c04fb2
e8d8c39
562ceb9
4ae30e4
062b90c
02f293e
1eb70d3
77185da
62531cc
bd683bc
2fc10c4
86e497c
53b2d2c
86b61de
9b042f1
bbbed6a
9be4479
d550c6b
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 |
---|---|---|
|
@@ -10,7 +10,7 @@ import * as ReactDOM from "react-dom"; | |
|
||
import { AbstractPureComponent } from "../../common/abstractPureComponent"; | ||
import * as Classes from "../../common/classes"; | ||
import { TOASTER_WARN_INLINE, TOASTER_WARN_LEFT_RIGHT } from "../../common/errors"; | ||
import { TOASTER_WARN_INLINE } from "../../common/errors"; | ||
import { ESCAPE } from "../../common/keys"; | ||
import { Position } from "../../common/position"; | ||
import { IProps } from "../../common/props"; | ||
|
@@ -19,6 +19,13 @@ import { Overlay } from "../overlay/overlay"; | |
import { IToastProps, Toast } from "./toast"; | ||
|
||
export type IToastOptions = IToastProps & { key?: string }; | ||
export type ToasterPosition = | ||
| Position.TOP | ||
| Position.TOP_LEFT | ||
| Position.TOP_RIGHT | ||
| Position.BOTTOM | ||
| Position.BOTTOM_LEFT | ||
| Position.BOTTOM_RIGHT; | ||
|
||
export interface IToaster { | ||
/** Show a new toast to the user. Returns the unique key of the new toast. */ | ||
|
@@ -66,11 +73,13 @@ export interface IToasterProps extends IProps { | |
inline?: boolean; | ||
|
||
/** | ||
* Position of `Toaster` within its container. Note that `LEFT` and `RIGHT` are disallowed | ||
* because Toaster only supports the top and bottom edges. | ||
* Position of `Toaster` within its container. | ||
* | ||
* Note that only `TOP` and `BOTTOM` are supported because Toaster only | ||
* supports the top and bottom edge positioning. | ||
* @default Position.TOP | ||
*/ | ||
position?: Position; | ||
position?: ToasterPosition; | ||
} | ||
|
||
export interface IToasterState { | ||
|
@@ -163,12 +172,6 @@ export class Toaster extends AbstractPureComponent<IToasterProps, IToasterState> | |
); | ||
} | ||
|
||
protected validateProps(props: IToasterProps) { | ||
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. Why remove this? 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. The validation is no longer valid because I changed the prop typing. |
||
if (props.position === Position.LEFT || props.position === Position.RIGHT) { | ||
console.warn(TOASTER_WARN_LEFT_RIGHT); | ||
} | ||
} | ||
|
||
private renderToast(toast: IToastOptions) { | ||
return <Toast {...toast} onDismiss={this.getDismissHandler(toast)} />; | ||
} | ||
|
@@ -179,7 +182,7 @@ export class Toaster extends AbstractPureComponent<IToasterProps, IToasterState> | |
} | ||
|
||
private getPositionClasses() { | ||
const positions = Position[this.props.position].split("_"); | ||
const positions = this.props.position.split("-"); | ||
// NOTE that there is no -center class because that's the default style | ||
return positions.map(p => `${Classes.TOAST_CONTAINER}-${p.toLowerCase()}`); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,17 +4,23 @@ | |
* Licensed under the terms of the LICENSE file distributed with this project. | ||
*/ | ||
|
||
/** | ||
* Enumeration of calendar months. | ||
* | ||
* Note that the enum values are numbers (with January as `0`) so they can be | ||
* easily compared to `date.getMonth()`. | ||
*/ | ||
export const enum Months { | ||
JANUARY, | ||
FEBRUARY, | ||
MARCH, | ||
APRIL, | ||
MAY, | ||
JUNE, | ||
JULY, | ||
AUGUST, | ||
SEPTEMBER, | ||
OCTOBER, | ||
NOVEMBER, | ||
DECEMBER, | ||
JANUARY = 0, | ||
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. Why numbers and not strings? 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. So, take a look at the comment. It explicitly mentions comparing with Date.getMonth() which uses zero-indexed months. 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. Oh, whoops, missed the comment. Good good. |
||
FEBRUARY = 1, | ||
MARCH = 2, | ||
APRIL = 3, | ||
MAY = 4, | ||
JUNE = 5, | ||
JULY = 6, | ||
AUGUST = 7, | ||
SEPTEMBER = 8, | ||
OCTOBER = 9, | ||
NOVEMBER = 10, | ||
DECEMBER = 11, | ||
} |
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.
Not that this is your commit at all, but why do we even have a default argument when we treat it as though nothing was passed?
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.
intent = "none"
is technically not the same asintent = null
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.
/shrug