-
-
Notifications
You must be signed in to change notification settings - Fork 0
Feature/consult 5055 #271
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
Feature/consult 5055 #271
Conversation
…ser, organization, data sources, and maps
… improve InspectorPanel tab management
…ate related components
…gTab for boundary configuration management
…undaryDataPanel for boundary data visualization
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.
@joaquimds if you ignore this file in terms of the review. It's just something I got copilot to generate, and which I think is nice to have there as a bog standard overview of the state of the database.
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 introduces a configurable inspector panel system for map views, enabling users to customize which boundary data is displayed and how it's organized. The changes add database persistence for inspector configurations, new React components for configuration UI and data display, and a refactored inspector panel with tabbed navigation.
Changes:
- Added database migration and schema updates to store inspector configurations per map view
- Implemented new configuration components for selecting data sources and columns to display in the inspector
- Refactored InspectorPanel to support multiple tabs (data, config, markers, notes) with dynamic sizing
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| migrations/1764611637231_map_view_inspector_config.ts | Adds inspector_config JSONB column to map_view table |
| src/server/services/database/schema.ts | Updates MapView interface documentation with new inspector_config field |
| src/server/models/MapView.ts | Defines InspectorConfig and InspectorBoundaryConfig schemas |
| src/shadcn/ui/multi-select.tsx | New multi-select component for choosing columns |
| src/app/map/[id]/components/TogglePanel.tsx | New collapsible panel component for UI organization |
| src/app/map/[id]/components/inspector/InspectorPanel.tsx | Refactored to support tabs and dynamic sizing |
| src/app/map/[id]/components/inspector/InspectorConfigTab.tsx | Configuration UI for boundary data display settings |
| src/app/map/[id]/components/inspector/InspectorDataTab.tsx | Data display tab with support for configured boundaries |
| src/app/map/[id]/components/inspector/BoundaryConfigItem.tsx | Configuration form for individual boundary data sources |
| src/app/map/[id]/components/inspector/BoundaryDataPanel.tsx | Displays boundary data based on configuration |
| src/app/map/[id]/components/inspector/InspectorMarkersTab.tsx | Extracted markers tab component |
| src/app/map/[id]/components/inspector/InspectorNotesTab.tsx | Placeholder notes tab component |
| src/app/map/[id]/hooks/useInitialMapView.ts | Initializes new views with empty inspector config |
| src/app/map/[id]/components/MapViews.tsx | Sets default inspector config when creating views |
| src/components/DataSourceItem.tsx | UI simplification with commented-out code |
| src/app/map/[id]/page.tsx | Added server-side debug logging |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export function BoundaryDataPanel({ | ||
| config, | ||
| dataSourceId, | ||
| areaCode, | ||
| columns, | ||
| defaultExpanded, | ||
| }: { | ||
| config: { name: string; dataSourceId: string }; | ||
| dataSourceId: string; | ||
| areaCode: string; | ||
| columns: string[]; | ||
| defaultExpanded: boolean; | ||
| }) { |
Copilot
AI
Jan 15, 2026
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.
The config parameter type is loosely defined as an object with name and dataSourceId, but the actual InspectorBoundaryConfig type has additional required fields (type and columns). This creates a type mismatch - the function should accept the full InspectorBoundaryConfig type to ensure type safety.
| )} | ||
| /> | ||
|
|
||
| {Icon} |
Copilot
AI
Jan 15, 2026
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.
The icon parameter is typed as React.ReactNode but is being conditionally rendered directly in JSX. When undefined, it will render nothing, which is fine. However, for consistency with the TogglePanelProps interface where rightIconButton is typed as LucideIcon, consider typing icon more specifically or adding a comment explaining that undefined icons are expected and handled correctly.
| <button | ||
| className="ml-1 rounded-full outline-none ring-offset-background focus:ring-2 focus:ring-ring focus:ring-offset-2" | ||
| onKeyDown={(e) => { | ||
| if (e.key === "Enter") { | ||
| handleUnselect(value); | ||
| } | ||
| }} | ||
| onMouseDown={(e) => { | ||
| e.preventDefault(); | ||
| e.stopPropagation(); | ||
| }} | ||
| onClick={() => handleUnselect(value)} | ||
| > | ||
| <X className="h-3 w-3 text-muted-foreground hover:text-foreground" /> | ||
| </button> |
Copilot
AI
Jan 15, 2026
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.
The remove button on the badge lacks an accessible label. Screen reader users won't know what this button does. Add an aria-label attribute such as aria-label={Remove ${option?.label}} to improve accessibility.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
…DataSourceById; fix initialization logic for boundaries
|
@ev-sc Approved, merge when ready! So good!!!! |
This pull request introduces a new, configurable inspector panel for map views, enabling users to customize which data is displayed and how it is organized for boundaries and other map features. The changes include a database migration for storing inspector configurations, new React components for configuration and data display, and a refactor of the inspector panel to support multiple tabs and improved UI flexibility.
Inspector panel redesign and configuration:
InspectorPanelto support multiple tabs (data,config,markers,notes) and improved layout logic, allowing users to switch between data display, configuration, marker lists, and notes. The panel now adapts its size and content based on the active tab. (src/app/map/[id]/components/inspector/InspectorPanel.tsx) (src/app/map/[id]/components/inspector/InspectorPanel.tsxL1-R10, src/app/map/[id]/components/inspector/InspectorPanel.tsxR19, src/app/map/[id]/components/inspector/InspectorPanel.tsxL38-L87)InspectorConfigTab, which allows users to configure boundary data display settings per map view. This tab initializes boundary configs if missing and provides UI for adding/editing boundary configs. (src/app/map/[id]/components/inspector/InspectorConfigTab.tsx) (src/app/map/[id]/components/inspector/InspectorConfigTab.tsxR1-R75)TogglePanelcomponent for collapsible panel sections, used throughout the inspector for better organization and UX. (src/app/map/[id]/components/TogglePanel.tsx) (src/app/map/[id]/components/TogglePanel.tsxR1-R67)Data display and boundary configuration:
BoundaryConfigItemandBoundaryDataPanelcomponents to support configuration and display of boundary-specific data, including selection of columns and data sources, and dynamic loading of data for selected boundaries. (src/app/map/[id]/components/inspector/BoundaryConfigItem.tsx,src/app/map/[id]/components/inspector/BoundaryDataPanel.tsx) (src/app/map/[id]/components/inspector/BoundaryConfigItem.tsxR1-R162, src/app/map/[id]/components/inspector/BoundaryDataPanel.tsxR1-R80)InspectorDataTabto use the new boundary configuration system, showing data panels for each configured boundary and falling back to default data display if no config is present. (src/app/map/[id]/components/inspector/InspectorDataTab.tsx) (src/app/map/[id]/components/inspector/InspectorDataTab.tsxR1-R172)Database and model changes:
inspector_configJSONB column to themap_viewdatabase table, enabling persistent storage of inspector configuration per map view. (migrations/1764611637231_map_view_inspector_config.ts)inspectorConfigcontaining an empty boundaries array. (src/app/map/[id]/components/MapViews.tsx) (src/app/map/[id]/components/MapViews.tsxR91)Additional tab functionality:
InspectorMarkersTab) and notes (InspectorNotesTab), laying the groundwork for future extensions. (src/app/map/[id]/components/inspector/InspectorMarkersTab.tsx,src/app/map/[id]/components/inspector/InspectorNotesTab.tsx) (src/app/map/[id]/components/inspector/InspectorMarkersTab.tsxR1-R18, src/app/map/[id]/components/inspector/InspectorNotesTab.tsxR1-R7)