-
Notifications
You must be signed in to change notification settings - Fork 0
PointsPanel #48
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
PointsPanel #48
Conversation
3196220 to
f866da1
Compare
1acff77 to
9f21be8
Compare
ccfd04c to
70e16ce
Compare
…get rid of radix dep)
Co-authored-by: Christophe Avenel <cavenel@users.noreply.github.com>
…g.groupBy.map into projectMap and map, fix config type guards, add ColorPicker based on react-colorful, add ColorControl (wip)
70e16ce to
0dbddb8
Compare
…ndcss refactor: switch to custom dockview theme
|
As discussed offline, since this branch affects more than just the points panel, we'll merge and continue development on |
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.
Pull request overview
This pull request implements a comprehensive PointsPanel with color and size configuration controls. The PR refactors several core APIs to support dynamic query-based column selection, updates configuration types to better separate project-level and custom mappings, and introduces new UI components for user interaction.
Changes:
- Refactored table/points data interfaces from synchronous
getColumns/getDimensionsto asyncsuggestColumnQueries/getColumnandsuggestDimensionQueries/getDimensionfor better query-based filtering - Updated
GroupByConfigAPI to use optionalprojectMapandmapfields (either/or) instead of a union type - Implemented ColorConfigControl and SizeConfigControl components with support for constant, from, groupBy, and random sources
- Added react-colorful dependency and created ColorPicker component for color selection
- Refactored ColorUtils to use
fromHex/toHexmethods instead of genericparseColor - Created new common UI components (accordion, field, toggle, radio-group, popover, simple-select, simple-combobox) following Base UI patterns
- Updated store slices to include
updateXmethods for partial updates - Implemented custom Tailwind CSS theme for dockview
Reviewed changes
Copilot reviewed 48 out of 49 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Added react-colorful@5.6.1 dependency |
| packages/@tissuumaps-viewer/src/components/Viewer/index.tsx | Simplified props to accept only className |
| packages/@tissuumaps-storage/src/table/TablePointsData.ts | Replaced getDimensions with async suggestDimensionQueries/getDimension methods |
| packages/@tissuumaps-storage/src/parquet/ParquetTableData.ts | Replaced getColumns with async suggestColumnQueries/getColumn methods |
| packages/@tissuumaps-storage/src/csv/CSVTableData.ts | Same as Parquet, plus added unused getColumns method |
| packages/@tissuumaps-core/src/utils/LoadUtils.ts | Updated to use projectMap field and improved color index clamping |
| packages/@tissuumaps-core/src/utils/ColorUtils.ts | Refactored parseColor to separate fromHex/toHex methods with validation |
| packages/@tissuumaps-core/src/utils/ColorUtils.test.ts | Updated tests to cover new fromHex/toHex methods |
| packages/@tissuumaps-core/src/storage/table.ts | Updated TableData interface with new async query methods |
| packages/@tissuumaps-core/src/storage/points.ts | Updated PointsData interface with new async query methods |
| packages/@tissuumaps-core/src/model/configs.ts | Refactored GroupByConfig to use optional projectMap/map fields, simplified type guards |
| packages/@tissuumaps-core/src/assets/shaders/points.frag | Fixed type conversion from float to vec2 |
| apps/tissuumaps/src/store/* | Added updateX methods to all store slices for partial updates |
| apps/tissuumaps/src/dockview.css | Created custom Tailwind-themed dockview CSS |
| apps/tissuumaps/src/components/ui/* | Added new UI primitives (toggle, toggle-group, radio-group, popover) and removed unused ones |
| apps/tissuumaps/src/components/panels/PointsPanel/* | Implemented PointsPanel with accordion UI and settings control |
| apps/tissuumaps/src/components/panels/ProjectPanel/index.tsx | Migrated to new field component structure |
| apps/tissuumaps/src/components/panels/ImagesPanel/index.tsx | Updated to use new accordion components |
| apps/tissuumaps/src/components/controls/ColorConfigControl/* | New control for managing color configurations with multiple sources |
| apps/tissuumaps/src/components/controls/SizeConfigControl/* | New control for managing size configurations with multiple sources |
| apps/tissuumaps/src/components/common/* | New common components (accordion, field, color-picker, simple-select, simple-combobox) |
| apps/tissuumaps/src/App.tsx | Simplified dockview integration with new theme approach |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)
packages/@tissuumaps-core/src/utils/LoadUtils.ts:83
- The logic for handling
projectMapandmaphas a potential issue. WhenprojectMapis undefined, the code checks ifmarkerConfig.groupBy.mapexists and uses it (line 82). However, according to the updated type definition inconfigs.ts, themapfield is now optional (map?: ValueMap<TValue>). If neitherprojectMapnormapis provided, the code will proceed to usemarkerMapwhich will beundefined, causing it to fall back to hashing logic. This behavior should be validated to ensure it's intentional, as it may be confusing that an incomplete configuration doesn't produce an error or warning.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
apps/tissuumaps/src/components/controls/ColorConfigControl/ColorConfigContextProvider.tsx
Show resolved
Hide resolved
apps/tissuumaps/src/components/controls/SizeConfigControl/SizeConfigContextProvider.tsx
Show resolved
Hide resolved
|
@jwindhager I've opened a new pull request, #55, to work on those changes. Once the pull request is ready, I'll request review from you. |
* Initial plan * Fix typo: TailwinCSS -> TailwindCSS in dockview.css comment Co-authored-by: jwindhager <7105759+jwindhager@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jwindhager <7105759+jwindhager@users.noreply.github.com>
|
@jwindhager I've opened a new pull request, #56, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
As discussed with @jwindhager, I will merge this branch and continue the development of the points and other tabs in the https://github.com/TissUUmaps/TissUUmaps4/tree/ui branch. |
|
@jwindhager I've opened a new pull request, #57, to work on those changes. Once the pull request is ready, I'll request review from you. |
No description provided.