Table plugin ts#96
Conversation
lint
handle bool arrays as categorical
✅ Deploy Preview for biongff-vizarr ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Fixed error in test runner, which was caused by the transitive dependency wgsl_reflect (deck.gl/core -> luma.gl/engine -> luma.gl/shadertools -> wgsl_reflect). I added an override to the package.json file to prevent updating to the breaking 1.3.0. The shadertools version we are depending on is 2 years old and could do with updating, which would be a better solution (e.g. upgrading deck.gl version). Output of pnpm why wgsl_reflect: dependencies: @deck.gl/core 9.0.41 |
| const sourceUrl = viewerInfo.sourceUrl ?? ""; | ||
|
|
||
| /** Navigate the viewer to a saved ROI (XY + Z) and make it visible. */ | ||
| const handleGoToSavedRoi = (roi: SavedRoi) => { |
There was a problem hiding this comment.
@AlexSurtees Tested out the impact on the ROI plugin as requested. Every core functionality works fine including the import of ROIs saved in the zarr!
Only one minor feature is impacted, which is the 'Go to ROI' one. It can still toggle visibility, and move Z/T axis, but the 'zooming in' feature is not working anymore. The write channel the ROI plugin calls to move the camera silently fails since setViewState was removed from onViewerStateChange in VizarrViewer.tsx, so at runtime the property is just undefined
I believe this feature to not be crucial but still useful so I would suggest fixing this.
There was a problem hiding this comment.
Thanks @LucaAnce. For the purposes of this PR, I think we will have to make a decision here about which is the lesser of two evils:
- Infinite re-render issue which does not seem to cause any functionality issues, but will no doubt affect performance of the viewer/browser.
- Ability for 'Go to ROI' functionality to match the correct zoom level for the ROI.
@davehorsfall what do you think?
There was a problem hiding this comment.
Understood, I missed the reason why that was removed. With this infinite re-render issue in mind, I understand your concerns and potentially support the removal of the go-to feature if no other solution is found.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Description
Fixes # (issue)
Related to # (if applicable)
#58
#83
Type of change
Checklist
npm test)pre-commit run --all-files)