feat(dashboard): add selector widget type (issue #1381) - #1504
Conversation
Adds a category dropdown / chip-list widget that shows distinct values from a layer field. Supports single-select (default) and multi-select mode. This is the UI foundation for cross-filtering (issue opengeos#1381 part 2): the selection state lives in the widget card, ready to feed a filter bus. Changes: - DashboardWidgetType: add "selector" - DashboardWidget: add optional multiple flag - WidgetEditorDialog: category picker + multi-select checkbox - DashboardPanel: SelectorValues chip list component - i18n: chartType.selector + editor.multiSelect keys
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new dashboard ChangesDashboard selector widgets
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant WidgetEditorDialog
participant DashboardPanel
participant AppState
participant distinctCategoryValues
participant SelectorValues
WidgetEditorDialog->>DashboardPanel: save selector configuration
DashboardPanel->>AppState: replaceWidget with complete record
AppState-->>DashboardPanel: return stored widget
DashboardPanel->>distinctCategoryValues: extract values from chart rows
distinctCategoryValues-->>DashboardPanel: return sorted distinct values
DashboardPanel->>SelectorValues: render values and selection mode
SelectorValues->>SelectorValues: toggle selected values
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
/claude-review |
🔍 Cloudflare PR preview
|
🔍 GitHub Pages PR preview
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/geolibre-desktop/src/components/panels/DashboardPanel.tsx`:
- Around line 568-579: Add aria-pressed={isSelected} to the selector chip button
in the toggle rendering, preserving the existing toggle behavior and styling so
assistive technology can identify each chip’s selected state.
- Around line 547-560: Update SelectorValues so its selected state is reset or
normalized whenever the multiple prop changes, ensuring single mode cannot
retain multiple selections from the prior mode. Prefer remounting via a key tied
to the selector configuration or add an effect that preserves at most one
selected value when multiple becomes false.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f2d56fe0-ea1f-488a-82df-961a2cd35051
📒 Files selected for processing (4)
apps/geolibre-desktop/src/components/panels/DashboardPanel.tsxapps/geolibre-desktop/src/components/panels/WidgetEditorDialog.tsxapps/geolibre-desktop/src/i18n/locales/en.jsonpackages/core/src/types.ts
- Key SelectorValues on the selector config (category + multiple) so the widget remounts with an empty selection when the field or single/multi mode changes, instead of leaving a stale multi-selection visible in single mode. - Add aria-pressed to the selector chips so assistive technology can report which values are selected, rather than conveying it through styling alone.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/geolibre-desktop/src/components/panels/DashboardPanel.tsx`:
- Around line 525-534: Update the SelectorValues key in the widget rendering
logic to include widget.layerId alongside the existing category and
multiple-mode values, ensuring the component remounts and clears its selection
whenever the source layer changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 10b46ab0-c11d-477d-9a07-6b62a816c5c5
📒 Files selected for processing (1)
apps/geolibre-desktop/src/components/panels/DashboardPanel.tsx
Verified against Natural Earth 110m countries (177 features) in the running
app: the CONTINENT field yields its 8 distinct values, REGION_UN its 6.
- Read category values from `row.properties[field]` instead of indexing the
row itself. `ChartRow` is `{ properties }`, so the old access returned
undefined for every feature, leaving the widget permanently showing its
"This layer has no chartable attributes." fallback — no chips ever
rendered. A double cast to `Record<string, unknown>` had hidden the
mismatch from the compiler.
- Extract the extraction into `distinctCategoryValues` in attribute-charts so
the row shape is covered by unit tests rather than an inline cast.
- Always write `multiple` when saving a selector. `updateWidget` merges its
patch onto the stored widget, so omitting the key when the box was
unchecked left an earlier `true` in place: switching multi-select off
silently did nothing and the widget stayed in multi mode.
- Add regression tests for the row shape, blank/nullish filtering, and the
merge semantics that made the omitted flag stick.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/geolibre-desktop/src/lib/attribute-charts.ts`:
- Around line 54-55: Update the category-value filtering in the attribute chart
value collection to test value.trim() rather than only value !== "", while
continuing to add the original value so nonblank display text is preserved. Add
a regression test covering whitespace-only property values and verify they are
excluded.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1d8bc074-6b26-490b-8138-be0bde78f227
📒 Files selected for processing (4)
apps/geolibre-desktop/src/components/panels/DashboardPanel.tsxapps/geolibre-desktop/src/components/panels/WidgetEditorDialog.tsxapps/geolibre-desktop/src/lib/attribute-charts.tstests/dashboard-widgets.test.ts
Editing a widget could not clear an optional field, and selector widgets did not survive a save at all. - Add a `replaceWidget` store action and use it when saving an edited widget. The editor hands back a complete record and omits the optional fields left empty, but `updateWidget` merges its patch, so an emptied title, color, prefix, or suffix kept its previous value and the change silently reverted when the dialog closed. Replacing also drops fields left over from the widget's previous type. `updateWidget` keeps its partial-patch semantics for every other caller. - Add "selector" to the widget-type allow-list in normalizeWidgets. It was missing, so every selector widget was discarded on save and never came back on reload. - Spell that allow-list as a Record keyed by DashboardWidgetType so a new member fails to compile until it is listed. The previous array was annotated rather than checked, which is how the omission went unnoticed. - Persist the selector's `multiple` flag in normalizeWidgets, which dropped it even once the type was accepted, and revert the editor to the file's omit-when-falsy style now that saving replaces rather than merges. - Cover the round-trip, the allow-list, and replace-vs-merge semantics. Verified in the app against Natural Earth 110m countries: setting a title, prefix, suffix, and color and then clearing each one now sticks, where the merging path left the old title in place.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/dashboard-widgets.test.ts`:
- Around line 190-192: Update the assertions around each widget lookup in the
test to assign the result of find to a saved variable, call assert.ok(saved)
before property checks, and then inspect saved directly without ?? {}. Apply
this pattern consistently to the cases at the current assertion and the
corresponding widget checks near the other referenced locations, preserving the
existing field expectations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6ebe4844-277d-4b98-ac66-1bd6677a7e21
📒 Files selected for processing (5)
apps/geolibre-desktop/src/components/panels/DashboardPanel.tsxapps/geolibre-desktop/src/components/panels/WidgetEditorDialog.tsxpackages/core/src/project.tspackages/core/src/store.tstests/dashboard-widgets.test.ts
- Include the layer id in the SelectorValues key. Switching a widget to
another layer while the category field and single/multi mode stayed the
same reused the component and carried the old layer's selection over.
- Drop whitespace-only category values in distinctCategoryValues. Only ""
was excluded, so a value of " " rendered as an empty, unlabelled chip.
The original spelling is still what the chip displays, so a value with
meaningful padding keeps it.
- Replace the `find(...) ?? {}` fallbacks in the tests added by this branch
with assert.ok narrowing. The fallback made the `"field" in saved` checks
pass against an empty object, so a widget being dropped entirely — the
regression these tests exist to catch — would not have failed them. The
pre-existing assertions elsewhere in the file are left as they are.
A selector's chips highlighted but nothing consumed the selection, so the widget looked broken: picking a value changed nothing on the dashboard. - Lift each selector's chosen values into DashboardPanel, keyed by widget id, and make SelectorValues a controlled component. The selection has to live where sibling widgets can read it. - Add `filterRowsBySelections`, which narrows rows by the active selections: values within one selector are OR-ed, separate selectors are AND-ed, and a selector with nothing picked filters nothing. - Every chart and indicator widget now renders from the rows left by the other selectors bound to its layer. A selector reads unfiltered rows so its own chip list stays complete and a choice can always be undone. - Drop a widget's selection when an edit repoints it at another layer, field, or selection mode, and when the widget is removed. This replaces the key-based remount, which no longer resets anything now that the state lives in the panel. - Selections are deliberately not persisted to the project: they are a way of looking at the data, not part of it. Filtering the map layer itself stays out of scope, per opengeos#1381 part 4. Verified against Natural Earth 110m countries (177 features): a count indicator reads 177, then 37 for "5. Low income", 28 with "Africa" added, 51 with the income filter removed, and 177 once cleared — each matching the dataset. Multi-select Africa+Asia gives 98 (51+47), and a pie over REGION_UN drops from 6 slices to 2.
Cross-filtering only shows up in the *other* widgets, so on a dashboard
holding just a selector, clicking a value appeared to do nothing at all —
the chip highlight was the only feedback the widget could give.
- Report the matching feature count under the chips ("51 of 176 features"),
counting against every active selection on the layer, not just this one.
- Add a Clear action, so a selection can be dropped without hunting for the
chip that set it (in single mode that was the only way back).
- Mark the count aria-live: the chips convey selection, but nothing was
announcing its effect.
Verified with the Countries sample (remote GeoParquet, 176 features):
selecting Africa reads "51 of 176 features" and Asia "47 of 176", matching
a DuckDB count over the same parquet, and Clear returns the widget to
showing no count.
Adds a scrollable table widget showing top-N features from a layer with configurable columns, sort field/direction, and row limit. Completes the three widget types proposed in opengeos#1381 (after indicator opengeos#1392 and selector but does not yet filter other widgets. Changes: - DashboardWidgetType: add "list" - DashboardWidget: add listFields, sortBy, sortDir, limit fields - project.ts: add "selector" and "list" to DASHBOARD_WIDGET_TYPES (selector was missing from the validator in the previous PR) - WidgetEditorDialog: column checkboxes + sort by/dir + limit input - DashboardPanel: ListTable component with sortable rows - i18n: chartType.list + 6 editor keys (en)
|
Hi @giswqs, thank you for the feedback and for all the work you've put into this PR — the cross-filtering and selection display are fantastic additions! I've consolidated the list widget into this PR as you requested (closed #1508). The branch now includes:
I'll add screenshots demonstrating both widgets working together as soon as possible. |
|
/claude-review |
|
All findings have been posted as buffered inline comments. Here's the final summary. Code reviewBugs
Quality
Security / CLAUDE.md: nothing found. New UI strings use |
- packages/core/src/project.ts: normalizeWidgets now carries a list widget's listFields/sortBy/sortDir/limit. It also runs on the save path (projectFromStore), so those fields were dropped the moment a project was saved and the widget rendered its "no data" fallback on reopen. The row limit is clamped to the editor's [1, 500] range. - DashboardPanel.tsx: drop the `const rows = data.rows` shadowing in the list render branch so a list honors the outer memo's filterRowsBySelections and cross-filters like every other widget. - DashboardPanel.tsx: default title for a list widget now reads from the chosen columns instead of the internal layer id (the layer name is already the card subtitle). - DashboardPanel.tsx: a selector's "N of total" now measures both counts against the rows left by the *other* selectors, so the fraction cannot read as a share of the whole layer while another selector has narrowed it. - DashboardPanel.tsx: memoize the selector's distinct values and match counts so an unrelated re-render no longer repeats a full scan of the layer's rows. - tests/dashboard-widgets.test.ts: cover list-widget normalization (kept fields, dropped/clamped junk, dropped from a non-list widget) and a serialize/parse round-trip.
Summary
Adds the selector widget — the second of three proposed in #1381 (after the indicator widget in #1392).
A category dropdown / chip-list bound to a layer field. Selecting a value highlights it; in multi-select mode, multiple chips can be active at once. This is the UI foundation for cross-filtering — the selection state is ready to feed a filter signal bus.
What's included
DashboardWidgetType: added"selector"DashboardWidget: added optionalmultipleflag for multi-selectWidgetEditorDialog: category field picker + multi-select checkbox (reuses existingcategoryCols)DashboardPanel: newSelectorValuescomponent renders a scrollable chip listchartType.selector+editor.multiSelectkeys (en)Screenshot
The widget renders as a compact card with clickable value chips inside the dashboard grid, matching the existing widget card layout (title bar, layer name, edit/move/delete controls).
What's NOT included (intentionally)
Test plan
tsc --noEmitpasses on core and desktop (no new errors)Closes part of #1381.
Summary by CodeRabbit