From ba085672229a7539e16291724f0584360a6a32f1 Mon Sep 17 00:00:00 2001 From: Daniel Chromik Date: Fri, 18 Sep 2026 09:43:32 +0200 Subject: [PATCH] feat(piechart): radius configuration enabled Signed-off-by: Daniel Chromik --- docs/piechart/go-sdk.md | 52 ++--- docs/piechart/model.md | 26 ++- piechart/schemas/migrate/migrate.cue | 14 +- .../schemas/migrate/tests/basic/expected.json | 6 +- piechart/schemas/pie.cue | 10 +- .../tests/invalid/pie-numeric-radius.json | 9 + .../tests/invalid/pie-top-level-radius.json | 7 + .../invalid/pie-unsupported-visual-field.json | 10 + piechart/schemas/tests/valid/pie-legacy.json | 9 + .../tests/valid/pie-radius-pixels.json | 10 + piechart/schemas/tests/valid/pie.json | 7 +- piechart/sdk/go/options.go | 22 +- piechart/sdk/go/options_test.go | 122 +++++++++++ piechart/sdk/go/pie.go | 80 +++++--- piechart/sdk/go/public_api_test.go | 65 ++++++ piechart/src/PieChartBase.test.tsx | 67 +++++++ piechart/src/PieChartBase.tsx | 17 +- .../PieChartOptionsEditorSettings.test.tsx | 189 ++++++++++++++++++ .../src/PieChartOptionsEditorSettings.tsx | 168 +++++++++++++--- piechart/src/PieChartPanel.test.tsx | 65 ++++++ piechart/src/PieChartPanel.tsx | 8 +- piechart/src/pie-chart-model.ts | 32 ++- 22 files changed, 882 insertions(+), 113 deletions(-) create mode 100644 piechart/schemas/tests/invalid/pie-numeric-radius.json create mode 100644 piechart/schemas/tests/invalid/pie-top-level-radius.json create mode 100644 piechart/schemas/tests/invalid/pie-unsupported-visual-field.json create mode 100644 piechart/schemas/tests/valid/pie-legacy.json create mode 100644 piechart/schemas/tests/valid/pie-radius-pixels.json create mode 100644 piechart/sdk/go/options_test.go create mode 100644 piechart/sdk/go/public_api_test.go create mode 100644 piechart/src/PieChartBase.test.tsx create mode 100644 piechart/src/PieChartOptionsEditorSettings.test.tsx create mode 100644 piechart/src/PieChartPanel.test.tsx diff --git a/docs/piechart/go-sdk.md b/docs/piechart/go-sdk.md index e143061fa..90bc9041f 100644 --- a/docs/piechart/go-sdk.md +++ b/docs/piechart/go-sdk.md @@ -15,7 +15,8 @@ Need a list of options. ## Default options -- None +- Calculation: `last` +- Visual outer radius: `"90%"` ## Available options @@ -43,13 +44,29 @@ package main import pie "github.com/perses/plugins/piechart/sdk/go" pie.WithVisual(pie.Visual{ - Palette: pie.Palette{ - Mode: pie.AutoMode, // or pie.CategoricalMode - }, + OuterRadius: "90%", }) ``` -Define visual properties of the pie chart including color palette mode. +Define the pie chart's radii and colors. Radius values can be percentages or unitless pixel strings. + +```golang +pie.WithVisual(pie.Visual{ + InnerRadius: "40%", + OuterRadius: "90%", + ColorPalette: []string{"#3366cc", "#dc3912"}, +}) +``` + +Set `InnerRadius` to create a doughnut chart. When omitted, the chart renders as a pie. + +### WithShowLabels + +```golang +pie.WithShowLabels(true) +``` + +Show labels inside the pie chart segments. ### WithFormat @@ -69,33 +86,15 @@ pie.WithFormat(&common.Format{ Define the format for pie chart values. -### WithQuerySettings - -```golang -package main - -import pie "github.com/perses/plugins/piechart/sdk/go" - -pie.WithQuerySettings([]pie.QuerySettingsItem{ - { - QueryIndex: 0, - ColorMode: pie.FixedMode, - ColorValue: "#FF5733", - }, -}) -``` - -Define color settings for specific queries. Available color modes: `FixedMode`, `FixedSingleMode`. - ## Example ```golang package main import ( + "github.com/perses/perses/go-sdk/common" "github.com/perses/perses/go-sdk/dashboard" "github.com/perses/perses/go-sdk/panel" - "github.com/perses/perses/go-sdk/common" pie "github.com/perses/plugins/piechart/sdk/go" ) @@ -110,7 +109,8 @@ func main() { Size: pie.MediumSize, }), pie.WithVisual(pie.Visual{ - Palette: pie.Palette{Mode: pie.CategoricalMode}, + InnerRadius: "40%", + OuterRadius: "90%", }), pie.WithFormat(&common.Format{ Unit: &common.BytesUnit, @@ -120,5 +120,5 @@ func main() { ), ), ) -}``` +} ``` diff --git a/docs/piechart/model.md b/docs/piechart/model.md index d2152aca8..40ec5191b 100644 --- a/docs/piechart/model.md +++ b/docs/piechart/model.md @@ -8,7 +8,11 @@ spec: format: # Optional sort: # Optional mode: # Optional - radius: + showLabels: # Optional + visual: + innerRadius: # Optional + outerRadius: + colorPalette: # Optional ``` ## Legend-with-values specification @@ -22,3 +26,23 @@ See [common plugin definitions](https://perses.dev/perses/docs/plugins/common/#c ## Format specification See [common plugin definitions](https://perses.dev/perses/docs/plugins/common/#format-specification). + +## Radii + +`visual.outerRadius` is required and defaults to `"90%"`. Set `visual.innerRadius` to create a doughnut chart. +Radius values can be percentages or unitless pixel strings and are passed to Apache ECharts. + +Persisted charts using the former top-level `radius` and `colorPalette` fields remain supported. The editor moves their +color settings into `visual` the next time the chart is saved; the former numeric `radius` did not affect rendering, so +these charts retain the `"90%"` rendered default. + +```yaml +# A pie chart with an outer radius relative to the shorter panel edge. +visual: + outerRadius: "90%" + +# A doughnut chart with a pixel inner radius and percentage outer radius. +visual: + innerRadius: "40" + outerRadius: "90%" +``` diff --git a/piechart/schemas/migrate/migrate.cue b/piechart/schemas/migrate/migrate.cue index 8c4639f90..98081e21a 100644 --- a/piechart/schemas/migrate/migrate.cue +++ b/piechart/schemas/migrate/migrate.cue @@ -52,11 +52,13 @@ spec: { showLabels: true } - #colorMode: *#panel.fieldConfig.defaults.color.mode | null - if #colorMode == "shades" { - #mappedColor: *commonMigrate.#mapping.color[#panel.fieldConfig.defaults.color.fixedColor] | "#555555" - colorPalette: [#mappedColor] - } + visual: { + #colorMode: *#panel.fieldConfig.defaults.color.mode | null + if #colorMode == "shades" { + #mappedColor: *commonMigrate.#mapping.color[#panel.fieldConfig.defaults.color.fixedColor] | "#555555" + colorPalette: [#mappedColor] + } - radius: 50 + outerRadius: "90%" + } } diff --git a/piechart/schemas/migrate/tests/basic/expected.json b/piechart/schemas/migrate/tests/basic/expected.json index 5bb902294..32defb667 100644 --- a/piechart/schemas/migrate/tests/basic/expected.json +++ b/piechart/schemas/migrate/tests/basic/expected.json @@ -7,7 +7,9 @@ "position": "bottom" }, "showLabels": true, - "colorPalette": ["#e02f44"], - "radius": 50 + "visual": { + "colorPalette": ["#e02f44"], + "outerRadius": "90%" + } } } diff --git a/piechart/schemas/pie.cue b/piechart/schemas/pie.cue index 58595f41f..204ea4a92 100644 --- a/piechart/schemas/pie.cue +++ b/piechart/schemas/pie.cue @@ -30,7 +30,15 @@ spec: close({ format?: common.#format sort?: "asc" | "desc" mode?: "value" | "percentage" + showLabels?: bool - radius: number + + // Deprecated top-level visual options remain accepted for persisted dashboards. + radius?: number colorPalette?: [...string] + visual?: close({ + innerRadius?: string + outerRadius: string + colorPalette?: [...string] + }) }) diff --git a/piechart/schemas/tests/invalid/pie-numeric-radius.json b/piechart/schemas/tests/invalid/pie-numeric-radius.json new file mode 100644 index 000000000..d474776fa --- /dev/null +++ b/piechart/schemas/tests/invalid/pie-numeric-radius.json @@ -0,0 +1,9 @@ +{ + "kind": "PieChart", + "spec": { + "calculation": "last", + "visual": { + "outerRadius": 90 + } + } +} diff --git a/piechart/schemas/tests/invalid/pie-top-level-radius.json b/piechart/schemas/tests/invalid/pie-top-level-radius.json new file mode 100644 index 000000000..35cbbf7f1 --- /dev/null +++ b/piechart/schemas/tests/invalid/pie-top-level-radius.json @@ -0,0 +1,7 @@ +{ + "kind": "PieChart", + "spec": { + "calculation": "last", + "outerRadius": "50" + } +} diff --git a/piechart/schemas/tests/invalid/pie-unsupported-visual-field.json b/piechart/schemas/tests/invalid/pie-unsupported-visual-field.json new file mode 100644 index 000000000..65cdd2d54 --- /dev/null +++ b/piechart/schemas/tests/invalid/pie-unsupported-visual-field.json @@ -0,0 +1,10 @@ +{ + "kind": "PieChart", + "spec": { + "calculation": "last", + "visual": { + "outerRadius": "90%", + "lineWidth": 1 + } + } +} diff --git a/piechart/schemas/tests/valid/pie-legacy.json b/piechart/schemas/tests/valid/pie-legacy.json new file mode 100644 index 000000000..18370329c --- /dev/null +++ b/piechart/schemas/tests/valid/pie-legacy.json @@ -0,0 +1,9 @@ +{ + "kind": "PieChart", + "spec": { + "calculation": "last-number", + "radius": 50, + "showLabels": true, + "colorPalette": ["#3366cc", "#dc3912"] + } +} diff --git a/piechart/schemas/tests/valid/pie-radius-pixels.json b/piechart/schemas/tests/valid/pie-radius-pixels.json new file mode 100644 index 000000000..e8dbe9bec --- /dev/null +++ b/piechart/schemas/tests/valid/pie-radius-pixels.json @@ -0,0 +1,10 @@ +{ + "kind": "PieChart", + "spec": { + "calculation": "last-number", + "visual": { + "innerRadius": "40", + "outerRadius": "200" + } + } +} diff --git a/piechart/schemas/tests/valid/pie.json b/piechart/schemas/tests/valid/pie.json index 17b19f435..caf459b1f 100644 --- a/piechart/schemas/tests/valid/pie.json +++ b/piechart/schemas/tests/valid/pie.json @@ -12,7 +12,10 @@ }, "sort": "desc", "mode": "value", - "radius": 50, - "showLabels": true + "showLabels": true, + "visual": { + "innerRadius": "40%", + "outerRadius": "90%" + } } } diff --git a/piechart/sdk/go/options.go b/piechart/sdk/go/options.go index 3a3fc8058..c46987b17 100644 --- a/piechart/sdk/go/options.go +++ b/piechart/sdk/go/options.go @@ -29,23 +29,37 @@ func WithLegend(legend Legend) Option { } } -func WithVisual(visual Visual) Option { +func WithShowLabels(showLabels bool) Option { return func(builder *Builder) error { - builder.Visual = &visual + builder.ShowLabels = showLabels return nil } } -func WithFormat(format *common.Format) Option { +func WithVisual(visual Visual) Option { return func(builder *Builder) error { - builder.Format = format + if visual.OuterRadius == "" { + visual.OuterRadius = defaultOuterRadius + if builder.Visual != nil { + visual.OuterRadius = builder.Visual.OuterRadius + } + } + builder.Visual = &visual return nil } } +// WithQuerySettings is deprecated. PieChart does not support query-specific colors. func WithQuerySettings(querySettingsList []QuerySettingsItem) Option { return func(builder *Builder) error { builder.QuerySettings = &querySettingsList return nil } } + +func WithFormat(format *common.Format) Option { + return func(builder *Builder) error { + builder.Format = format + return nil + } +} diff --git a/piechart/sdk/go/options_test.go b/piechart/sdk/go/options_test.go new file mode 100644 index 000000000..a531ee3e4 --- /dev/null +++ b/piechart/sdk/go/options_test.go @@ -0,0 +1,122 @@ +// Copyright The Perses Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package pie + +import ( + "encoding/json" + "testing" + + "gopkg.in/yaml.v3" +) + +func TestVisualOuterRadiusDefaultsToNinetyPercent(t *testing.T) { + builder, err := create() + if err != nil { + t.Fatalf("create failed: %v", err) + } + + if builder.Visual.OuterRadius != "90%" { + t.Errorf("expected default outer radius to be 90%%, got %q", builder.Visual.OuterRadius) + } +} + +func TestWithVisualRetainsDefaultRadiusWhenOmitted(t *testing.T) { + builder, err := create(WithVisual(Visual{ColorPalette: []string{"#3366cc"}})) + if err != nil { + t.Fatalf("create failed: %v", err) + } + + if builder.Visual.OuterRadius != "90%" { + t.Errorf("expected default outer radius to be retained, got %q", builder.Visual.OuterRadius) + } +} + +func TestWithVisualSerializesPieOptions(t *testing.T) { + builder, err := create(WithVisual(Visual{ + InnerRadius: "40%", + OuterRadius: "90%", + ColorPalette: []string{"#3366cc", "#dc3912"}, + })) + if err != nil { + t.Fatalf("create failed: %v", err) + } + + bytes, err := json.Marshal(builder) + if err != nil { + t.Fatalf("marshal failed: %v", err) + } + + var result map[string]any + if err := json.Unmarshal(bytes, &result); err != nil { + t.Fatalf("unmarshal failed: %v", err) + } + + visual, ok := result["visual"].(map[string]any) + if !ok { + t.Fatalf("expected visual options, got %#v", result["visual"]) + } + if visual["innerRadius"] != "40%" || visual["outerRadius"] != "90%" { + t.Errorf("unexpected radii: %#v", visual) + } + colorPalette, ok := visual["colorPalette"].([]any) + if !ok || len(colorPalette) != 2 || colorPalette[0] != "#3366cc" || colorPalette[1] != "#dc3912" { + t.Errorf("unexpected color palette: %#v", visual["colorPalette"]) + } +} + +func TestWithShowLabelsSerializesAtTheTopLevel(t *testing.T) { + builder, err := create(WithShowLabels(true)) + if err != nil { + t.Fatalf("create failed: %v", err) + } + + bytes, err := json.Marshal(builder) + if err != nil { + t.Fatalf("marshal failed: %v", err) + } + + var result map[string]any + if err := json.Unmarshal(bytes, &result); err != nil { + t.Fatalf("unmarshal failed: %v", err) + } + if result["showLabels"] != true { + t.Errorf("expected labels to be enabled, got %#v", result["showLabels"]) + } + visual, ok := result["visual"].(map[string]any) + if !ok { + t.Fatalf("expected visual options, got %#v", result["visual"]) + } + if _, exists := visual["showLabels"]; exists { + t.Errorf("expected labels outside visual options, got %#v", visual) + } +} + +func TestWithVisualSerializesRadiiAsYAML(t *testing.T) { + builder, err := create(WithVisual(Visual{ + InnerRadius: "40%", + OuterRadius: "90%", + })) + if err != nil { + t.Fatalf("create failed: %v", err) + } + + bytes, err := yaml.Marshal(builder) + if err != nil { + t.Fatalf("marshal failed: %v", err) + } + + if string(bytes) != "calculation: last\nvisual:\n innerRadius: 40%\n outerRadius: 90%\n" { + t.Errorf("unexpected YAML: %s", bytes) + } +} diff --git a/piechart/sdk/go/pie.go b/piechart/sdk/go/pie.go index 04076cd75..feeaec001 100644 --- a/piechart/sdk/go/pie.go +++ b/piechart/sdk/go/pie.go @@ -18,7 +18,10 @@ import ( "github.com/perses/perses/go-sdk/panel" ) -const PluginKind = "PieChart" +const ( + PluginKind = "PieChart" + defaultOuterRadius = "90%" +) type LegendPosition string @@ -48,6 +51,22 @@ type Legend struct { Values []common.Calculation `json:"values,omitempty" yaml:"values,omitempty"` } +type Sort string + +const ( + AscendingSort Sort = "asc" + DescendingSort Sort = "desc" +) + +type PluginMode string + +const ( + ValueMode PluginMode = "value" + PercentageMode PluginMode = "percentage" +) + +// Deprecated: these options were inherited from a different chart model and +// are ignored when serializing PieChart visual settings. type PaletteMode string const ( @@ -55,10 +74,12 @@ const ( CategoricalMode PaletteMode = "categorical" ) +// Deprecated: use Visual.ColorPalette. type Palette struct { - Mode PaletteMode `json:"mode" yaml:"mode"` + Mode PaletteMode `json:"-" yaml:"-"` } +// Deprecated: retained for source compatibility. type VisualDisplay string const ( @@ -66,6 +87,7 @@ const ( BarDisplay VisualDisplay = "bar" ) +// Deprecated: retained for source compatibility. type VisualShowPoints string const ( @@ -73,6 +95,7 @@ const ( AlwaysShowPoints VisualShowPoints = "always" ) +// Deprecated: retained for source compatibility. type VisualStack string const ( @@ -80,17 +103,7 @@ const ( PercentageStack VisualStack = "percent" ) -type Visual struct { - Display VisualDisplay `json:"display,omitempty" yaml:"display,omitempty"` - LineWidth float64 `json:"lineWidth,omitempty" yaml:"lineWidth,omitempty"` - AreaOpacity float64 `json:"areaOpacity,omitempty" yaml:"areaOpacity,omitempty"` - ShowPoints VisualShowPoints `json:"showPoints,omitempty" yaml:"showPoints,omitempty"` - Palette Palette `json:"palette,omitempty" yaml:"palette,omitempty"` - PointRadius float64 `json:"pointRadius,omitempty" yaml:"pointRadius,omitempty"` - Stack VisualStack `json:"stack,omitempty" yaml:"stack,omitempty"` - ConnectNulls bool `json:"connectNulls,omitempty" yaml:"connectNulls,omitempty"` -} - +// Deprecated: retained for source compatibility. type ColorMode string const ( @@ -98,37 +111,41 @@ const ( FixedSingleMode ColorMode = "fixed-single" ) +// Deprecated: query-specific colors are not supported by PieChart. type QuerySettingsItem struct { - QueryIndex uint `json:"queryIndex" yaml:"queryIndex"` - ColorMode ColorMode `json:"colorMode" yaml:"colorMode"` - ColorValue string `json:"colorValue" yaml:"colorValue"` + QueryIndex uint `json:"-" yaml:"-"` + ColorMode ColorMode `json:"-" yaml:"-"` + ColorValue string `json:"-" yaml:"-"` } -type Sort string - -const ( - AscendingSort Sort = "asc" - DescendingSort Sort = "desc" -) - -type PluginMode string - -const ( - ValueMode PluginMode = "value" - PercentageMode PluginMode = "percentage" -) +type Visual struct { + InnerRadius string `json:"innerRadius,omitempty" yaml:"innerRadius,omitempty"` + OuterRadius string `json:"outerRadius" yaml:"outerRadius"` + ColorPalette []string `json:"colorPalette,omitempty" yaml:"colorPalette,omitempty"` + + // Deprecated fields retained for source compatibility. They are not PieChart settings. + Display VisualDisplay `json:"-" yaml:"-"` + LineWidth float64 `json:"-" yaml:"-"` + AreaOpacity float64 `json:"-" yaml:"-"` + ShowPoints VisualShowPoints `json:"-" yaml:"-"` + Palette Palette `json:"-" yaml:"-"` + PointRadius float64 `json:"-" yaml:"-"` + Stack VisualStack `json:"-" yaml:"-"` + ConnectNulls bool `json:"-" yaml:"-"` +} type Option func(plugin *Builder) error type PluginSpec struct { Legend *Legend `json:"legend,omitempty" yaml:"legend,omitempty"` - QuerySettings *[]QuerySettingsItem `json:"querySettings,omitempty" yaml:"querySettings,omitempty"` Calculation common.Calculation `json:"calculation" yaml:"calculation"` Format *common.Format `json:"format,omitempty" yaml:"format,omitempty"` Sort Sort `json:"sort,omitempty" yaml:"sort,omitempty"` Mode PluginMode `json:"mode,omitempty" yaml:"mode,omitempty"` + ShowLabels bool `json:"showLabels,omitempty" yaml:"showLabels,omitempty"` Visual *Visual `json:"visual,omitempty" yaml:"visual,omitempty"` - Radius int `json:"radius" yaml:"radius"` + Radius int `json:"radius,omitempty" yaml:"radius,omitempty"` + QuerySettings *[]QuerySettingsItem `json:"-" yaml:"-"` } func create(options ...Option) (Builder, error) { @@ -138,6 +155,7 @@ func create(options ...Option) (Builder, error) { defaults := []Option{ Calculation(common.LastCalculation), + WithVisual(Visual{OuterRadius: defaultOuterRadius}), } for _, opt := range append(defaults, options...) { diff --git a/piechart/sdk/go/public_api_test.go b/piechart/sdk/go/public_api_test.go new file mode 100644 index 000000000..c0ccd63b7 --- /dev/null +++ b/piechart/sdk/go/public_api_test.go @@ -0,0 +1,65 @@ +// Copyright The Perses Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package pie_test + +import ( + "encoding/json" + "testing" + + "github.com/perses/perses/go-sdk/common" + pie "github.com/perses/plugins/piechart/sdk/go" +) + +func TestVisualRadiiAreUsableThroughThePublicSDK(t *testing.T) { + spec := pie.PluginSpec{ + Calculation: common.LastCalculation, + ShowLabels: true, + Visual: &pie.Visual{ + InnerRadius: "40%", + OuterRadius: "90%", + }, + } + + bytes, err := json.Marshal(spec) + if err != nil { + t.Fatalf("marshal failed: %v", err) + } + + if string(bytes) != `{"calculation":"last","showLabels":true,"visual":{"innerRadius":"40%","outerRadius":"90%"}}` { + t.Errorf("unexpected JSON: %s", bytes) + } +} + +func TestLegacyVisualAPIStillCompiles(t *testing.T) { + visual := pie.Visual{ + Display: pie.LineDisplay, + LineWidth: 2, + AreaOpacity: 0.5, + ShowPoints: pie.AutoShowPoints, + Palette: pie.Palette{Mode: pie.AutoMode}, + PointRadius: 3, + Stack: pie.AllStack, + ConnectNulls: true, + } + querySettings := []pie.QuerySettingsItem{{ + QueryIndex: 0, + ColorMode: pie.FixedMode, + ColorValue: "#3366cc", + }} + legacySpec := pie.PluginSpec{Radius: 50, Visual: &visual, QuerySettings: &querySettings} + + if legacySpec.Visual == nil || pie.WithVisual(visual) == nil || pie.WithQuerySettings(querySettings) == nil { + t.Fatal("expected legacy options to remain usable") + } +} diff --git a/piechart/src/PieChartBase.test.tsx b/piechart/src/PieChartBase.test.tsx new file mode 100644 index 000000000..c69efed33 --- /dev/null +++ b/piechart/src/PieChartBase.test.tsx @@ -0,0 +1,67 @@ +// Copyright The Perses Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import { ChartsProvider, testChartsTheme } from '@perses-dev/components'; +import type * as Components from '@perses-dev/components'; +import { render } from '@testing-library/react'; + +import type { PieChartData } from './PieChartBase'; +import { PieChartBase } from './PieChartBase'; + +const { eChartSpy } = vi.hoisted(() => ({ eChartSpy: vi.fn() })); + +vi.mock('@perses-dev/components', async (importOriginal) => { + const components = await importOriginal(); + return { + ...components, + EChart: (props: unknown): null => { + eChartSpy(props); + return null; + }, + }; +}); + +interface EChartProps { + option: { + series: Array<{ + radius: string | [string, string]; + }>; + }; +} + +const DONUT_RADIUS: [string, string] = ['40%', '90%']; +const EMPTY_DATA: PieChartData[] = []; + +describe('PieChartBase', () => { + it('passes a scalar outer radius to ECharts for a pie chart', () => { + render( + + + , + ); + + const props = eChartSpy.mock.lastCall?.[0] as EChartProps; + expect(props.option.series[0]?.radius).toBe('75%'); + }); + + it('passes a doughnut radius to ECharts', () => { + render( + + + , + ); + + const props = eChartSpy.mock.lastCall?.[0] as EChartProps; + expect(props.option.series[0]?.radius).toEqual(DONUT_RADIUS); + }); +}); diff --git a/piechart/src/PieChartBase.tsx b/piechart/src/PieChartBase.tsx index b2ec6a0d6..330ea6ee6 100644 --- a/piechart/src/PieChartBase.tsx +++ b/piechart/src/PieChartBase.tsx @@ -20,6 +20,7 @@ import { use as registerECharts } from 'echarts/core'; import { CanvasRenderer } from 'echarts/renderers'; import type { ReactElement } from 'react'; +import { DEFAULT_OUTER_RADIUS } from './pie-chart-model'; import { getLabelFormatter, getTooltipFormatter } from './utils'; registerECharts([ @@ -47,12 +48,24 @@ export interface PieChartBaseProps { mode?: ModeOption; showLabels?: boolean; formatOptions?: FormatOptions; + innerRadius?: string; + outerRadius?: string; } export function PieChartBase(props: PieChartBaseProps): ReactElement { - const { width, height, data, mode, formatOptions, showLabels } = props; + const { + width, + height, + data, + mode, + formatOptions, + showLabels, + innerRadius, + outerRadius = DEFAULT_OUTER_RADIUS, + } = props; const chartsTheme = useChartsTheme(); const muiTheme = useTheme(); + const radius = innerRadius ? [innerRadius, outerRadius] : outerRadius; const option = { tooltip: { @@ -64,7 +77,7 @@ export function PieChartBase(props: PieChartBaseProps): ReactElement { series: [ { type: 'pie', - radius: '90%', + radius, label: { show: Boolean(showLabels), position: 'inner', diff --git a/piechart/src/PieChartOptionsEditorSettings.test.tsx b/piechart/src/PieChartOptionsEditorSettings.test.tsx new file mode 100644 index 000000000..d2f14979f --- /dev/null +++ b/piechart/src/PieChartOptionsEditorSettings.test.tsx @@ -0,0 +1,189 @@ +// Copyright The Perses Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import { ChartsProvider, testChartsTheme } from '@perses-dev/components'; +import { fireEvent, render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import type { ReactElement } from 'react'; +import { useCallback, useState } from 'react'; + +import { createInitialPieChartOptions, DEFAULT_VISUAL } from './pie-chart-model'; +import type { PieChartOptions, PieChartVisualOptions } from './pie-chart-model'; +import { PieChartOptionsEditorSettings } from './PieChartOptionsEditorSettings'; + +function renderEditor(value: PieChartOptions = createInitialPieChartOptions(), onChange = vi.fn()): void { + render( + + + , + ); +} + +interface ControlledEditorProps { + onChange: (value: PieChartOptions) => void; +} + +function ControlledEditor({ onChange }: ControlledEditorProps): ReactElement { + const [value, setValue] = useState(createInitialPieChartOptions); + + const handleChange = useCallback( + (nextValue: PieChartOptions): void => { + setValue(nextValue); + onChange(nextValue); + }, + [onChange], + ); + + return ; +} + +function renderControlledEditor(onChange = vi.fn()): void { + render( + + + , + ); +} + +function getOuterRadius(): HTMLInputElement { + return screen.getByRole('textbox', { name: 'Outer Radius' }); +} + +function getInnerRadius(): HTMLInputElement { + return screen.getByRole('textbox', { name: 'Inner Radius' }); +} + +function withVisual(visual: Partial): PieChartOptions { + const initialOptions = createInitialPieChartOptions(); + return { + ...initialOptions, + visual: { + ...DEFAULT_VISUAL, + ...visual, + }, + }; +} + +describe('PieChartOptionsEditorSettings', () => { + it('renders a legacy chart without visual settings', () => { + const legacyOptions: PieChartOptions = { + ...createInitialPieChartOptions(), + visual: undefined, + }; + + expect(() => renderEditor(legacyOptions)).not.toThrow(); + }); + + it('moves legacy visual fields when saving a legacy chart', () => { + const onChange = vi.fn(); + const legacyOptions: PieChartOptions & { radius: number; showLabels: boolean; colorPalette: string[] } = { + ...createInitialPieChartOptions(), + visual: undefined, + radius: 50, + showLabels: true, + colorPalette: ['#3366cc', '#dc3912'], + }; + renderEditor(legacyOptions, onChange); + + fireEvent.change(getOuterRadius(), { target: { value: '75%' } }); + + expect(onChange).toHaveBeenLastCalledWith({ + calculation: legacyOptions.calculation, + format: legacyOptions.format, + mode: legacyOptions.mode, + showLabels: true, + sort: legacyOptions.sort, + visual: { + outerRadius: '75%', + colorPalette: ['#3366cc', '#dc3912'], + }, + }); + }); + + it('shows the default outer radius and no inner radius', () => { + renderEditor(); + + expect(getOuterRadius()).toHaveValue('90%'); + expect(getInnerRadius()).toHaveValue(''); + }); + + it('stores a unitless outer radius', () => { + const onChange = vi.fn(); + renderEditor(createInitialPieChartOptions(), onChange); + + fireEvent.change(getOuterRadius(), { target: { value: '200' } }); + + expect(onChange).toHaveBeenLastCalledWith(withVisual({ outerRadius: '200' })); + }); + + it('keeps the outer radius focused while typing in a controlled editor', () => { + const onChange = vi.fn(); + renderControlledEditor(onChange); + + const outerRadius = getOuterRadius(); + userEvent.clear(outerRadius); + userEvent.type(outerRadius, '75%'); + + expect(outerRadius).toHaveFocus(); + expect(outerRadius).toHaveValue('75%'); + expect(onChange).toHaveBeenLastCalledWith(withVisual({ outerRadius: '75%' })); + }); + + it('stores an inner radius for a doughnut chart', () => { + const onChange = vi.fn(); + renderEditor(createInitialPieChartOptions(), onChange); + + fireEvent.change(getInnerRadius(), { target: { value: '40%' } }); + + expect(onChange).toHaveBeenLastCalledWith(withVisual({ innerRadius: '40%', outerRadius: '90%' })); + }); + + it('restores the default when the required outer radius is left blank', () => { + const onChange = vi.fn(); + renderEditor(createInitialPieChartOptions(), onChange); + + const outerRadius = getOuterRadius(); + fireEvent.change(outerRadius, { target: { value: '' } }); + fireEvent.blur(outerRadius); + + expect(onChange).toHaveBeenLastCalledWith(withVisual({ outerRadius: '90%' })); + }); + + it('displays unitless pixel radius values', () => { + renderEditor(withVisual({ innerRadius: '40', outerRadius: '90%' })); + + expect(getInnerRadius()).toHaveValue('40'); + expect(getOuterRadius()).toHaveValue('90%'); + }); + + it('resets visual settings to their defaults', () => { + const onChange = vi.fn(); + const value = { + ...withVisual({ + innerRadius: '40%', + outerRadius: '90%', + colorPalette: ['#3366cc'], + }), + showLabels: true, + }; + renderEditor(value, onChange); + + fireEvent.click(screen.getByRole('button', { name: 'Reset To Defaults' })); + + expect(onChange).toHaveBeenLastCalledWith({ + ...value, + showLabels: false, + visual: { ...DEFAULT_VISUAL }, + }); + }); +}); diff --git a/piechart/src/PieChartOptionsEditorSettings.tsx b/piechart/src/PieChartOptionsEditorSettings.tsx index c768c7190..f7b43e70f 100644 --- a/piechart/src/PieChartOptionsEditorSettings.tsx +++ b/piechart/src/PieChartOptionsEditorSettings.tsx @@ -12,7 +12,7 @@ // limitations under the License. import type { SwitchProps } from '@mui/material'; -import { Button, FormControl, InputLabel, MenuItem, Select, Stack, Switch, Typography } from '@mui/material'; +import { Button, FormControl, InputLabel, MenuItem, Select, Stack, Switch, TextField, Typography } from '@mui/material'; import type { FormatControlsProps, SortSelectorProps, @@ -39,18 +39,100 @@ import { CalculationSelector, LegendOptionsEditor } from '@perses-dev/plugin-sys import { produce } from 'immer'; import merge from 'lodash/merge'; import omit from 'lodash/omit'; -import type { ReactElement } from 'react'; -import { useMemo } from 'react'; +import type { ChangeEvent, ReactElement } from 'react'; +import { useCallback, useMemo, useState } from 'react'; -import type { PieChartOptions, PieChartOptionsEditorProps } from './pie-chart-model'; -import { DEFAULT_FORMAT } from './pie-chart-model'; +import { DEFAULT_FORMAT, DEFAULT_OUTER_RADIUS, DEFAULT_VISUAL, resolvePieChartVisualOptions } from './pie-chart-model'; +import type { PieChartOptions, PieChartOptionsEditorProps, PieChartVisualOptions } from './pie-chart-model'; + +interface RadiusInputs { + innerRadius?: string; + outerRadius: string; +} + +interface RadiusControlsProps { + innerRadius?: string; + outerRadius: string; + onChange: (radius: RadiusInputs) => void; +} + +function RadiusControls({ innerRadius = '', outerRadius, onChange }: RadiusControlsProps): ReactElement { + const [isOuterRadiusEmpty, setIsOuterRadiusEmpty] = useState(false); + + const updateRadius = useCallback( + (inner: string, outer: string): void => { + if (outer === '') return; + + onChange({ innerRadius: inner || undefined, outerRadius: outer }); + }, + [onChange], + ); + + const handleOuterRadiusChange = useCallback( + (event: ChangeEvent): void => { + const outer = event.target.value; + if (outer === '') { + setIsOuterRadiusEmpty(true); + return; + } + + setIsOuterRadiusEmpty(false); + updateRadius(innerRadius, outer); + }, + [innerRadius, updateRadius], + ); + + const handleInnerRadiusChange = useCallback( + (event: ChangeEvent): void => { + const outer = isOuterRadiusEmpty ? DEFAULT_OUTER_RADIUS : outerRadius; + setIsOuterRadiusEmpty(false); + updateRadius(event.target.value, outer); + }, + [isOuterRadiusEmpty, outerRadius, updateRadius], + ); + + const handleOuterRadiusBlur = useCallback((): void => { + if (!isOuterRadiusEmpty) return; + + setIsOuterRadiusEmpty(false); + updateRadius(innerRadius, DEFAULT_OUTER_RADIUS); + }, [innerRadius, isOuterRadiusEmpty, updateRadius]); + + return ( + + + + + ); +} + +function normalizePieChartOptions(value: PieChartOptions): PieChartOptions & { visual: PieChartVisualOptions } { + const normalizedValue = { ...value }; + delete normalizedValue.radius; + delete normalizedValue.colorPalette; + return { + ...normalizedValue, + visual: resolvePieChartVisualOptions(value), + }; +} export function PieChartOptionsEditorSettings(props: PieChartOptionsEditorProps): ReactElement { - const { onChange, value } = props; + const { onChange, value: persistedValue } = props; + const value: PieChartOptions & { visual: PieChartVisualOptions } = useMemo( + () => normalizePieChartOptions(persistedValue), + [persistedValue], + ); const handleCalculationChange: CalculationSelectorProps['onChange'] = (newCalculation: CalculationType) => { onChange( - produce(value, (draft: PieChartOptions) => { + produce(value, (draft) => { draft.calculation = newCalculation; }), ); @@ -58,7 +140,7 @@ export function PieChartOptionsEditorSettings(props: PieChartOptionsEditorProps) const handleLegendChange: LegendOptionsEditorProps['onChange'] = (newLegend) => { onChange( - produce(value, (draft: PieChartOptions) => { + produce(value, (draft) => { draft.legend = newLegend; }), ); @@ -66,7 +148,7 @@ export function PieChartOptionsEditorSettings(props: PieChartOptionsEditorProps) const handleUnitChange: FormatControlsProps['onChange'] = (newFormat: FormatOptions) => { onChange( - produce(value, (draft: PieChartOptions) => { + produce(value, (draft) => { draft.format = newFormat; }), ); @@ -74,7 +156,7 @@ export function PieChartOptionsEditorSettings(props: PieChartOptionsEditorProps) const handleSortChange: SortSelectorProps['onChange'] = (newSort: SortOption) => { onChange( - produce(value, (draft: PieChartOptions) => { + produce(value, (draft) => { draft.sort = newSort; }), ); @@ -82,7 +164,7 @@ export function PieChartOptionsEditorSettings(props: PieChartOptionsEditorProps) const handleModeChange: ModeSelectorProps['onChange'] = (newMode: ModeOption) => { onChange( - produce(value, (draft: PieChartOptions) => { + produce(value, (draft) => { draft.mode = newMode; }), ); @@ -90,28 +172,40 @@ export function PieChartOptionsEditorSettings(props: PieChartOptionsEditorProps) const handleShowLabelsChange: SwitchProps['onChange'] = (_: unknown, checked: boolean) => { onChange( - produce(value, (draft: PieChartOptions) => { + produce(value, (draft) => { draft.showLabels = checked; }), ); }; + const handleRadiusChange: RadiusControlsProps['onChange'] = useCallback( + ({ innerRadius, outerRadius }) => { + onChange( + produce(value, (draft) => { + draft.visual.innerRadius = innerRadius; + draft.visual.outerRadius = outerRadius; + }), + ); + }, + [onChange, value], + ); + const chartsTheme = useChartsTheme(); const themePalette = chartsTheme.echartsTheme.color; const colorPalette: string[] | undefined = useMemo(() => { - return value.colorPalette || undefined; - }, [value.colorPalette]); + return value.visual.colorPalette || undefined; + }, [value.visual.colorPalette]); const handleColorChange = (color?: string[]): void => { onChange( - produce(value, (draft: PieChartOptions) => { + produce(value, (draft) => { if (Array.isArray(color)) { - draft.colorPalette = color; + draft.visual.colorPalette = color; } else if (typeof color === 'string') { - draft.colorPalette = [color]; + draft.visual.colorPalette = [color]; } else { - draft.colorPalette = undefined; + draft.visual.colorPalette = undefined; } }), ); @@ -165,19 +259,12 @@ export function PieChartOptionsEditorSettings(props: PieChartOptionsEditorProps) - - } + + - - - - - - - - @@ -207,16 +294,31 @@ export function PieChartOptionsEditorSettings(props: PieChartOptionsEditorProps) )} + + + + } + /> + + + + + + +