feat(dashboard): add indicator widget type - #1392
Merged
giswqs merged 10 commits intoJul 25, 2026
Merged
Conversation
Add a KPI indicator tile to the dashboard panel alongside the existing chart widgets (histogram, scatter, bar, line, box, pie). Changes: - Add 'indicator' to DashboardWidgetType in types.ts - Add IndicatorAggregation type: count, sum, mean, min, max, median - Add indicator fields to DashboardWidget: indicatorAggregation, prefix, suffix (field is reused from existing schema) - Implement computeIndicator() and formatIndicatorValue() in DashboardPanel with a centered big-number tile render - Add indicator option to WidgetEditorDialog with aggregation picker, field selector (hidden for count), and prefix/suffix inputs - Register 'indicator' in DASHBOARD_WIDGET_TYPES in normalizeWidgets - Add INDICATOR_AGGREGATIONS validation array in project.ts - Add i18n keys for indicator chart type and aggregations (en, es) - Add tests: indicator round-trip, count without field, invalid aggregation rejection The indicator widget is the first step toward richer dashboard elements. Cross-filtering and selector/list widgets will follow in subsequent PRs.
normalizeString does not trim, so removing the explicit .trim() call on prefix/suffix preserves intentional leading/trailing spaces like ' ha' or '$ '.
❌ Deploy Preview for geolibre-app failed.
|
Contributor
📝 WalkthroughWalkthroughAdds an ChangesIndicator widget support
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant WidgetEditorDialog
participant normalizeWidgets
participant DashboardPanel
participant WidgetCard
WidgetEditorDialog->>normalizeWidgets: save indicator configuration
normalizeWidgets->>DashboardPanel: provide normalized widget
DashboardPanel->>WidgetCard: render indicator widget
WidgetCard->>WidgetCard: compute and format KPI value
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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/WidgetEditorDialog.tsx`:
- Line 230: Update the widget type selector fallback around the “indicator”
option so count indicators remain selectable when hasChartable is false. Keep
the selector available for layers without chartable columns, while disabling or
filtering only widget types that require a field; preserve the existing
indicator option and renderer behavior.
- Around line 147-156: Update the indicator save logic in WidgetEditorDialog to
preserve the original prefix and suffix whitespace instead of trimming them.
Store prefix and suffix values directly when saving, while retaining the
existing conditional behavior for empty values and other indicator fields.
🪄 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: c51e14ef-6e99-4835-803f-be8b88b7ab9d
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
apps/geolibre-desktop/src/components/panels/DashboardPanel.tsxapps/geolibre-desktop/src/components/panels/WidgetEditorDialog.tsxapps/geolibre-desktop/src/i18n/locales/en.jsonapps/geolibre-desktop/src/i18n/locales/es.jsonpackages/core/src/project.tspackages/core/src/types.tstests/dashboard-widgets.test.ts
- Preserve prefix/suffix whitespace in editor save (was trimming) - Allow count indicator on layers without chartable columns - Use DashboardWidgetType instead of ChartType in editor state (ChartType does not include 'indicator') - Remove unused ChartType import Tests: 27/27 pass.
Contributor
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/WidgetEditorDialog.tsx`:
- Around line 189-191: Update the WidgetEditorDialog rendering around the
hasChartable/type condition so the widget type selector remains visible when no
chartable fields exist. Disable only options that require fields, keep the
indicator option selectable, and render the noFields message separately without
replacing the selector.
🪄 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: 0f8a0b3e-6c4c-43bc-a78f-ccf64a1c5d51
📒 Files selected for processing (1)
apps/geolibre-desktop/src/components/panels/WidgetEditorDialog.tsx
Member
|
@bgoniasa Can you look into the CI errors and resolve them? |
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/src/project.ts (1)
815-827: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winRestrict indicator fields to indicator widgets.
normalizeWidgetscurrently preservesindicatorAggregation,prefix, andsuffixfor every widget type. Add atype === "indicator"guard so non-indicator widgets cannot round-trip invalid indicator configuration.Proposed fix
if ( + type === "indicator" && candidate.indicatorAggregation && INDICATOR_AGGREGATIONS.includes(candidate.indicatorAggregation) ) { widget.indicatorAggregation = candidate.indicatorAggregation; } - const prefix = normalizeString(candidate.prefix); - if (prefix) widget.prefix = prefix; - const suffix = normalizeString(candidate.suffix); - if (suffix) widget.suffix = suffix; + if (type === "indicator") { + const prefix = normalizeString(candidate.prefix); + if (prefix) widget.prefix = prefix; + const suffix = normalizeString(candidate.suffix); + if (suffix) widget.suffix = suffix; + }🤖 Prompt for 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. In `@packages/core/src/project.ts` around lines 815 - 827, Update normalizeWidgets around the indicatorAggregation, prefix, and suffix assignments to guard all three fields with candidate.type === "indicator". Preserve the existing aggregation validation and prefix/suffix normalization for indicator widgets, while preventing non-indicator widgets from retaining these properties.
🤖 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.
Outside diff comments:
In `@packages/core/src/project.ts`:
- Around line 815-827: Update normalizeWidgets around the indicatorAggregation,
prefix, and suffix assignments to guard all three fields with candidate.type ===
"indicator". Preserve the existing aggregation validation and prefix/suffix
normalization for indicator widgets, while preventing non-indicator widgets from
retaining these properties.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 089835cd-08f0-4cc5-8c9f-73110fd7fce5
📒 Files selected for processing (4)
apps/geolibre-desktop/src/i18n/locales/en.jsonapps/geolibre-desktop/src/i18n/locales/es.jsonpackages/core/src/project.tspackages/core/src/types.ts
Member
|
Still failing.
|
- DashboardPanel: `widgetToSpec` now takes the narrowed `ChartType`, and the
chart is computed only for non-indicator widgets, fixing TS2322 ("indicator"
is not assignable to ChartType).
- DashboardPanel: `computeIndicator` takes `ChartRow[]` and reads values via the
shared `numericValues` helper. It was indexing the row object directly
(`row[field]`), but rows are `{ properties }`, so every non-count aggregation
returned null — this fixes TS2345 and the indicator always showing "No data".
- WidgetEditorDialog: keep the widget type selector visible when a layer has no
numeric or categorical columns, showing the "no fields" note alongside it.
A new widget defaults to "histogram", so the old branch hid the selector
before the user could reach the enabled "indicator" option and count
indicators were uncreatable on attribute-less layers.
- packages/core/project.ts: `normalizeWidgets` now guards
`indicatorAggregation`/`prefix`/`suffix` on `type === "indicator"` so
non-indicator widgets cannot round-trip dead indicator configuration; covered
by a new test.
Member
|
Pushed 71a8cac, which addresses the remaining review feedback and the failing build.
|
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/geolibre-desktop/src/components/panels/DashboardPanel.tsx (2)
472-498: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winGate the KPI on
data.hasData.Unlike the chart branch (Line 501), the indicator branch ignores
data.hasData. When the layer is missing or its data isn't loaded,data.rowsis empty and acountindicator renders a confident0instead of the no-data message — the header says "layer missing" while the tile reports a real-looking value.🐛 Proposed fix
{(() => { const agg = widget.indicatorAggregation ?? "count"; - const value = computeIndicator(data.rows, widget.field, agg); + const value = data.hasData ? computeIndicator(data.rows, widget.field, agg) : null; if (value === null) {🤖 Prompt for 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. In `@apps/geolibre-desktop/src/components/panels/DashboardPanel.tsx` around lines 472 - 498, Update the indicator rendering branch in DashboardPanel to check data.hasData before computing or displaying the KPI value. When data.hasData is false, render the existing dashboard.noData message instead of calculating a count from empty data.rows; preserve the current aggregation, formatting, and styled value behavior when data is available.
50-53: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winAvoid spreading unbounded row values into
Math.min/Math.max.
valuescomes from a selected chart field, so large layers can pass enough arguments forMath.min(...values)/Math.max(...values)to throwRangeError. Use a finitereducefor min/max here; other code paths pass bounded inputs like coordinate ranges.🛡️ Proposed fix
case "min": - return Math.min(...values); + return values.reduce((a, b) => (b < a ? b : a)); case "max": - return Math.max(...values); + return values.reduce((a, b) => (b > a ? b : a));🤖 Prompt for 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. In `@apps/geolibre-desktop/src/components/panels/DashboardPanel.tsx` around lines 50 - 53, Update the "min" and "max" branches in the chart value aggregation switch to compute results with finite reduce operations instead of spreading values into Math.min or Math.max. Preserve the existing aggregation results for non-empty values and leave other aggregation branches unchanged.
🤖 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.
Outside diff comments:
In `@apps/geolibre-desktop/src/components/panels/DashboardPanel.tsx`:
- Around line 472-498: Update the indicator rendering branch in DashboardPanel
to check data.hasData before computing or displaying the KPI value. When
data.hasData is false, render the existing dashboard.noData message instead of
calculating a count from empty data.rows; preserve the current aggregation,
formatting, and styled value behavior when data is available.
- Around line 50-53: Update the "min" and "max" branches in the chart value
aggregation switch to compute results with finite reduce operations instead of
spreading values into Math.min or Math.max. Preserve the existing aggregation
results for non-empty values and leave other aggregation branches unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3cb494c6-cbb3-4184-9a37-78dff61b1b47
📒 Files selected for processing (4)
apps/geolibre-desktop/src/components/panels/DashboardPanel.tsxapps/geolibre-desktop/src/components/panels/WidgetEditorDialog.tsxpackages/core/src/project.tstests/dashboard-widgets.test.ts
Contributor
🔍 PR preview
|
giswqs
approved these changes
Jul 25, 2026
This was referenced Jul 28, 2026
bgoniasa
added a commit
to bgoniasa/GeoLibre
that referenced
this pull request
Jul 29, 2026
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)
giswqs
added a commit
that referenced
this pull request
Aug 1, 2026
* feat(dashboard): add selector widget type (issue #1381) 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 #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 * style: auto-format (ruff + oxfmt) [pre-commit.ci] * Address CodeRabbit review feedback - 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. * fix(dashboard): make the selector widget render and honor single mode 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. * fix(dashboard): persist cleared widget fields and selector widgets 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. * Address CodeRabbit review feedback - 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. * feat(dashboard): cross-filter widgets from selector selections 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 #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. * feat(dashboard): show what a selector selection matched 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. * feat(dashboard): add list widget type (issue #1381) 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 #1381 (after indicator #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) * style: auto-format (ruff + oxfmt) [pre-commit.ci] * Address Claude review feedback - 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. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Qiusheng Wu <giswqs@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds an indicator widget to the dashboard panel — a KPI tile showing a single aggregated value (count, sum, mean, min, max, median) as a large number with optional prefix/suffix. First PR from #1381.
What changed
Types (
packages/core/src/types.ts):"indicator"toDashboardWidgetTypeIndicatorAggregationtype:"count" | "sum" | "mean" | "min" | "max" | "median"indicatorAggregation,prefix, andsuffixfields toDashboardWidgetDashboard panel (
DashboardPanel.tsx):computeIndicator()— aggregates layer rows by the selected functionformatIndicatorValue()— locale-aware number formattingWidget editor (
WidgetEditorDialog.tsx):Persistence (
project.ts):"indicator"inDASHBOARD_WIDGET_TYPESINDICATOR_AGGREGATIONSvalidation arrayi18n (
en.json,es.json)Tests: All 27 tests pass (round-trip, count without field, invalid aggregation)
First step from #1381. Selector, list, and cross-filtering will follow.
Summary by CodeRabbit