Skip to content
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

Master sunburst chart adrm #5756

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Conversation

hokolomopo
Copy link
Contributor

@hokolomopo hokolomopo commented Feb 18, 2025

Description:

[IMP] demo: add demo data for sunburst chart

[IMP] chart: add sunburst chart

This commit adds a new chart type: sunburst chart. The sunburst chart
is used to display hierarchical data in a circular chart.

Technically, the chart is implemented using a doughnut chart with
multiple levels and invisible segments.

[IMP] chart: add ChartWithAxisDefinition type

Before this, we used the type ChartWithDataSetDefinition in the
component AxisDesignEditor. Which was wrong because there are charts
with datasets but no axis (geo, pie). It forced those chart to have a
unused key axisDesign in their definition.

The aggregated key in the geo chart definition is also never used
(they are always aggregated).

[IMP] tests: define default creation context for chart tests

Every type of chart has a test that checks what happens if we
create a chart from a creation context. But every time we add something
new to to creation context, we have to change every single test, even
if the chart doesn't use the new key of the context.

This commits adds a constant with a default creation context to
simplify the tests.

[IMP] chart: extract ChartTitle tools into TextStyler

This commit extract the style editing tools of the ChartTitle
component into a new component called TextStyler. This will be used
in the following commits to have a more generic component that can be
used to style any text in the chart.

It also adds add the possibility ot edit a background color or a
vertical alignment in this TextStyler component.

The commit also simplifies the TextStyler compared to the previous
behaviour of ChartTitle by:

  • deleting all the props updateColor, toggleBold, ... in favor of a
    single callback updateStyle
  • using ActionButton components instead of custom HTML that looks like
    an action button

[MOV] chart: rename title files to chart_title

Task: 4575651

review checklist

  • feature is organized in plugin, or UI components
  • support of duplicate sheet (deep copy)
  • in model/core: ranges are Range object, and can be adapted (adaptRanges)
  • in model/UI: ranges are strings (to show the user)
  • undo-able commands (uses this.history.update)
  • multiuser-able commands (has inverse commands and transformations where needed)
  • new/updated/removed commands are documented
  • exportable in excel
  • translations (_t("qmsdf %s", abc))
  • unit tested
  • clean commented code
  • track breaking changes
  • doc is rebuild (npm run doc)
  • status is correct in Odoo

@robodoo
Copy link
Collaborator

robodoo commented Feb 18, 2025

Pull request status dashboard

@hokolomopo hokolomopo force-pushed the master-sunburst-chart-adrm branch 7 times, most recently from 0baa01c to 44f0936 Compare March 5, 2025 07:38
This commit extract the style editing tools of the `ChartTitle`
component into a new component called `TextStyler`. This will be used
in the following commits to have a more generic component that can be
used to style any text in the chart.

It also adds add the possibility ot edit a background color or a
vertical alignment in this `TextStyler` component.

The commit also simplifies the TextStyler compared to the previous
behaviour of `ChartTitle` by:

- deleting all the props updateColor, toggleBold, ... in favor of a
single callback updateStyle
- using ActionButton components instead of custom HTML that looks like
an action button

Task: 4575651
Every type of chart has a test that checks what happens if we
create a chart from a creation context. But every time we add something
new to to creation context, we have to change every single test, even
if the chart doesn't use the new key of the context.

This commits adds a constant with a default creation context to
simplify the tests.

Task: 4575651
Before this, we used the type `ChartWithDataSetDefinition` in the
component `AxisDesignEditor`. Which was wrong because there are charts
with datasets but no axis (geo, pie). It forced those chart to have a
unused key `axisDesign` in their definition.

The `aggregated` key in the geo chart definition is also never used
(they are always aggregated).

Task: 4575651
@hokolomopo hokolomopo force-pushed the master-sunburst-chart-adrm branch 4 times, most recently from 4d85a3e to 06a3900 Compare March 17, 2025 09:54
@hokolomopo hokolomopo force-pushed the master-sunburst-chart-adrm branch 3 times, most recently from 3f0ff3d to 75de4cd Compare March 20, 2025 13:32
This commit adds a new chart type: sunburst chart. The sunburst chart
is used to display hierarchical data in a circular chart.

Technically, the chart is implemented using a doughnut chart with
multiple levels and invisible segments.

Task: 4575651
When hovering a sunburst item, an offset is added to the pie slice, but
the position of the label was not updated accordingly.

Task: 4575651
With this commit, the children of the hovered item in the sunburst
chart are highlighted in addition to the hovered item, and the other
elements are dimmed.

This is done by adding a new plugin to the `ChartJS`. The plugin
`sunburstHoverPlugin` make the children of the hovered item
`active`, so they get the hovered offset and color.

Task: 4575651
@hokolomopo hokolomopo force-pushed the master-sunburst-chart-adrm branch from 75de4cd to c6c0fc8 Compare March 21, 2025 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants