-
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
add new lint rules to ease migration #2394
Changes from 10 commits
fbc5fb1
f31f098
1bab25e
f5cf2d4
086914c
cd9bdab
0eed2a8
8f7de99
c425b97
537f519
efa54b3
3ecccfa
ebd4ff8
6254e7d
0ae1f97
1ce4d8d
bdb5735
1925ebf
b90c870
e7e6661
c039df2
92ebaa6
689aa67
f96fc1b
2ecd03a
82189f0
b7f7008
6f31a13
5480d98
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 |
---|---|---|
|
@@ -336,11 +336,11 @@ describe("<DateInput>", () => { | |
wrapper.setState({ isOpen: true }); | ||
|
||
// try typing a new time | ||
wrapper.find(".pt-timepicker-millisecond").simulate("change", { target: { value: "1" } }); | ||
wrapper.find("." + Classes.TIMEPICKER_MILLISECOND).simulate("change", { target: { value: "1" } }); | ||
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. nit: we use backtick interpolation elsewhere. might as well be consistent. |
||
assert.isTrue(wrapper.find(Popover).prop("isOpen")); | ||
|
||
// try keyboard-incrementing to a new time | ||
wrapper.find(".pt-timepicker-millisecond").simulate("keydown", { which: Keys.ARROW_UP }); | ||
wrapper.find("." + Classes.TIMEPICKER_MILLISECOND).simulate("keydown", { which: Keys.ARROW_UP }); | ||
assert.isTrue(wrapper.find(Popover).prop("isOpen")); | ||
}); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -52,7 +52,7 @@ export class GuideLayer extends React.Component<IGuideLayerProps, {}> { | |
left: `${offset}px`, | ||
}; | ||
const className = classNames(Classes.TABLE_OVERLAY, Classes.TABLE_VERTICAL_GUIDE, { | ||
"pt-table-vertical-guide-flush-left": offset === 0, | ||
[`${Classes.TABLE_VERTICAL_GUIDE}-flush-left`]: offset === 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. nit: since our classes have configurable PREFIX, then this will technically work, but it's worth noting this is slightly different that the other uses since we're building a classname string. |
||
}); | ||
return <div className={className} key={index} style={style} />; | ||
}; | ||
|
@@ -62,7 +62,7 @@ export class GuideLayer extends React.Component<IGuideLayerProps, {}> { | |
top: `${offset}px`, | ||
}; | ||
const className = classNames(Classes.TABLE_OVERLAY, Classes.TABLE_HORIZONTAL_GUIDE, { | ||
"pt-table-horizontal-guide-flush-top": offset === 0, | ||
[`${Classes.TABLE_HORIZONTAL_GUIDE}-flush-top`]: offset === 0, | ||
}); | ||
return <div className={className} key={index} style={style} />; | ||
}; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,4 @@ | ||
.npmrc | ||
scripts/ | ||
src/ | ||
test/ |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,6 +13,8 @@ module.exports = { | |
|
||
defaultSeverity: "error", | ||
|
||
rulesDirectory: "./rules", | ||
|
||
rules: { | ||
"ban": { | ||
options: [ | ||
|
@@ -25,6 +27,8 @@ module.exports = { | |
// ["assert", "equal", "use assert.strictEqual instead"] | ||
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. this file should extend 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. no i thought that was the whole point of this... that the base config does not include any blueprint-specific stuff? 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. nah, the purpose is the inverse of that actually, that you can use the blueprint-specific rules (such as no icon strings) without pulling in the rest of the config 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. ok so default config includes blueprint, but you can also just use blueprint without config? 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. yep! because most apps using BP are going to want to pick up the BP rules and config that say "don't use the .pt- prefix" but not pick up the general TS lint rule config |
||
], | ||
}, | ||
"blueprint-classes-constants": true, | ||
"blueprint-icon-components": false, | ||
"file-header": { | ||
options: [ | ||
"Copyright \\d{4} Palantir Technologies, Inc\\. All rights reserved.", | ||
|
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.
will this also fail if the class can't be found? (i'm trying to infer why
assert.isTrue
was used...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.
"assert that the spinner is not spinning"