feat(heatmap): SDS Heatmap init #546
Conversation
tihuan
force-pushed
the
thuang-heatmap
branch
2 times, most recently
from
e6728ba
to
0d05af2
Compare
|
Wow, @tihuan, this is such a big deal 😯🤩🎉 Thank you very much for putting forth the effort to make this happen! I'm pretty excited to get started on it. I still need to go over the PR. I want to go over everything thoroughly to completely understand the API you established. Thank you a lot. 🙏🏻❤️ |
|
Thanks, Masoud!! I'll try to publish the heatmap to npm this week and import it in single cell to use it! And in the meantime if it's easier for me to walkthrough the code with you, we can just schedule a zoom call! |
tihuan
force-pushed
the
thuang-heatmap
branch
3 times, most recently
from
bad0acc
to
96f8952
Compare
| const path = require("path"); | ||
|
|
||
| /** | ||
| * (thuang): Inspired by https://github.com/cwmoo740/jest-svg-transformer/issues/3#issuecomment-1229928189 |
There was a problem hiding this comment.
this replaces svg-jest, since that package doesn't work with the latest jest, due to jest's API changes
.yarn/releases/yarn-1.19.0.js
Outdated
There was a problem hiding this comment.
I had to lock yarn version to 1.19.0 to avoid installation issue
| @@ -0,0 +1,20 @@ | |||
| // Mock IntersectionObserver | |||
There was a problem hiding this comment.
upgraded jest and jest-dom no longer has intersectionObserver mock, so we need to create one ourselves
| @@ -0,0 +1,16 @@ | |||
| module.exports = { | |||
There was a problem hiding this comment.
extract a common config file for reuse
| @@ -78,9 +79,10 @@ | |||
| "eslint-plugin-sonarjs": "^0.19.0", | |||
| "eslint-plugin-storybook": "^0.6.13", | |||
| "husky": "^5.1.3", | |||
| "jest": "^27.2.5", | |||
| "jest": "^29.6.2", | |||
There was a problem hiding this comment.
upgrade jest to the latest version!
| }, | ||
| }; | ||
|
|
||
| export interface CreateChartOptionsProps { |
There was a problem hiding this comment.
these are the selected echarts API that we document and expose as part of the SDS HeatmapChart API
| const customGrid = | ||
| typeof gridProp === "function" ? gridProp(defaultGrid) : gridProp; | ||
|
|
||
| return { |
There was a problem hiding this comment.
a lot more echarts default configs we set for users here, but once we add HeatmapChartProps.configs, users will be able to customize all the echarts options
|
|
||
| if (onChartMouseMove) { | ||
| rawChart.getZr().on("mousemove", function (event: ElementEvent) { | ||
| onChartMouseMove(event, rawChart); |
There was a problem hiding this comment.
onChartMouseMove is not in its final API form yet. The ideal API should expose the underlying item, itemIndex, etc. as described in the API doc:
This is how the current onChartMouseMove API is being used on the SC side. Note that SC still needs to use low level echarts API to convert screen pixel coordinates to actual item index, which the final API will do the conversion for the SDS users
| "files": [ | ||
| "src/core/Heatmap/Heatmap.namespace-test.tsx", | ||
| "include": [ | ||
| "src/core/**/*.namespace-test.tsx", |
There was a problem hiding this comment.
using wildcard is easier!
| "src/core/TooltipCondensed/TooltipCondensed.namespace-test.tsx", | ||
| "src/core/TooltipTable/TooltipTable.namespace-test.tsx", | ||
| "include": [ | ||
| "src/core/**/*.namespace-test.tsx", |
There was a problem hiding this comment.
using wildcard is easier!
| 'data-file-name': ${name}.name, | ||
| 'data-testid': ${name}.name, |
There was a problem hiding this comment.
only add these two props for backward compatibility. The snapshot files are updated, since we no longer have prop data-jest-svg-name. E.g., data-jest-svg-name="IconChevronDownSmall"
|
@masoudmanson I was able to verify that the component works in Single Cell by installing it locally 🚀 ! How should we publish this lol? Thank you! |
There was a problem hiding this comment.
Thank you for this thorough and well-written PR, @tihuan. I went over everything, and it looks good to me. 🤩💯
The release process still has some bugs and does not work correctly; once you merge this PR, I can manually release the new packages. |
|
Amazing! Thanks so much for the thorough review, @masoudmanson 🙏 Merging now and appreciate you deploying this! Please let me know when it's done and I'll use it in Single Cell 🚀 🤩 |
Summary
Structural Element (Base, Gene, DNA, Chromosome or Cell)
Github issue: #XXXX
Copy isuue descirption here
Checklist
defaultTheme.tsused wherever possible