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

[FIX] selection input: highlight color not sync with range color #5734

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hokolomopo
Copy link
Contributor

Description

After re-ordering the selection inputs of a chart, the highlight colors would not be the same as the range colors.

This was because the store mutator updateColors would not update the store range colors. And store.highlights is based on ranges.colors while store.selectionInputs ignores the colors of store.ranges.

The test update of colors are taken into account was also a bit sketchy. We would modify the props.colors array in place, thus the condition nextProps.colors !== props.colors would never be satisfied, and store.updateColors would never be called.

Task: 4577803

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 14, 2025

Pull request status dashboard

After re-ordering the selection inputs of a chart, the highlight colors
would not be the same as the range colors.

This was because the store mutator `updateColors` would not update
the store range colors. And `store.highlights` is based on
`ranges.colors` while `store.selectionInputs` ignores the colors
of `store.ranges`.

The test `update of colors are taken into account` was also a bit
sketchy. We would modify the `props.colors` array in place, thus
the condition `nextProps.colors !== props.colors` would never
be satisfied, and `store.updateColors` would never be called.

Task: 4577803
@hokolomopo hokolomopo force-pushed the master-fix-selection-input-reorder-colors-adrm branch from c683699 to 99fa619 Compare February 14, 2025 10:51
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.

2 participants