-
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
Fix #2095: Hot reload Table #2390
Fix #2095: Hot reload Table #2390
Conversation
Thanks for your interest in palantir/blueprint, @dan-katz! Before we can accept your pull request, you need to sign our contributor license agreement - just visit https://cla.palantir.com/ and follow the instructions. Once you sign, I'll automatically update this pull request. |
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.
nice stuff, but just reuse the util function exported from core instead of copy-paste.
packages/table/src/common/utils.ts
Outdated
@@ -329,6 +331,10 @@ export const Utils = { | |||
const approxCellHeight = approxNumLinesDesired * approxLineHeight; | |||
return approxCellHeight; | |||
}, | |||
|
|||
isElementOfType<P = {}>(element: any, ComponentClass: React.ComponentClass<P>): element is React.ReactElement<P> { |
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.
🙅♂️ import { Utils } from "@blueprintjs/core"
instead of copying code
packages/table/src/cell/cell.tsx
Outdated
@@ -10,6 +10,7 @@ import * as Classes from "../common/classes"; | |||
import { Classes as CoreClasses, IIntentProps, IProps, Utils as CoreUtils } from "@blueprintjs/core"; | |||
|
|||
import { LoadableContent } from "../common/loadableContent"; | |||
import { Utils } from "../common/utils"; |
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.
import { Utils } from "@blueprintjs/core"
use isElementOfType util function from corePreview: documentation | landing | table |
fix linting errorsPreview: documentation | landing | table |
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.
👍 nice work @dan-katz!
whohooo!!! Thanks @dan-katz ! |
Fixes #2095
Checklist
Changes proposed in this pull request:
Add the isElementOfType function from PR 2099 to the Table package and use it for element comparison in the Table and Cell components.
Reviewers should focus on:
Ensuring that the Table and Cell components still work as expected