Enum Stringification #1921
Enum Stringification #1921
Conversation
|
Blocked on Popover2 merge since tetherUtils (failing compilation) will be deleted |
| @@ -202,8 +202,8 @@ export function iconClass(iconName?: string) { | |||
| } | |||
|
|
|||
| export function intentClass(intent = Intent.NONE) { | |||
There was a problem hiding this comment.
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.
intent = "none" is technically not the same as intent = null
| @@ -163,12 +165,6 @@ export class Toaster extends AbstractPureComponent<IToasterProps, IToasterState> | |||
| ); | |||
| } | |||
|
|
|||
| protected validateProps(props: IToasterProps) { | |||
There was a problem hiding this comment.
The validation is no longer valid because I changed the prop typing.
| OCTOBER, | ||
| NOVEMBER, | ||
| DECEMBER, | ||
| JANUARY = 0, |
There was a problem hiding this comment.
Why numbers and not strings?
There was a problem hiding this comment.
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.
Oh, whoops, missed the comment. Good good.
| OCTOBER, | ||
| NOVEMBER, | ||
| DECEMBER, | ||
| JANUARY = 0, |
There was a problem hiding this comment.
Oh, whoops, missed the comment. Good good.
packages/core/src/common/position.ts
Outdated
| BOTTOM_LEFT = "bottom_left", | ||
| LEFT_BOTTOM = "left_bottom", | ||
| LEFT = "left", | ||
| LEFT_TOP = "left_top", |
There was a problem hiding this comment.
convention elsewhere is dashes for lowercase strings. left-top
| * @default Position.TOP | ||
| */ | ||
| position?: Position; | ||
| position?: Position.TOP | Position.BOTTOM; |
There was a problem hiding this comment.
this is not complete. it's actually TOP_* and BOTTOM_*: can align left/right too.
| @@ -179,7 +175,7 @@ export class Toaster extends AbstractPureComponent<IToasterProps, IToasterState> | |||
| } | |||
|
|
|||
| private getPositionClasses() { | |||
| const positions = Position[this.props.position].split("_"); | |||
| const positions = this.props.position.split("_"); | |||
There was a problem hiding this comment.
be sure to update this with the change to -
| */ | ||
| TOP_LEFT, | ||
| TOP_LEFT = "top_left", |
|
are you using
|
|
Please merge master to get the new required status check |
|
I did enable auto-lint on save but it's not working properly for some reason |
Fix more docs examplesPreview: documentation | table |
Merge branch 'develop' into gg/string-enumsPreview: documentation | table |
For all exported enums, supply explicit enum values (preferably strings) so that users can use fully qualified enum object or simple string constants.
Some enums use numerical values for functional purposes -- these were not changed (e.g. Month, Elevation).