Skip to content

Conversation

@hokolomopo
Copy link
Contributor

Description

This commits adds the "top10" conditional formatting operator, which highlights the top or bottom N values in a selected range.

Task: 5367065

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 Nov 28, 2025

Pull request status dashboard

Comment on lines +116 to +123
const evaluator = criterionEvaluatorRegistry.get(cf.rule.operator);
const criterion = { ...cf.rule, type: cf.rule.operator };
const ranges = cf.ranges.map((xc) => this.getters.getRangeFromSheetXC(sheetId, xc));
const preComputedCriterion = evaluator.preComputeCriterion?.(
criterion,
ranges,
this.getters
);
Copy link
Contributor Author

@hokolomopo hokolomopo Nov 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a huge fan, but top10 do a sort to get the top X values, and we really don't wanna sort the range once for every cell of the range, it will explode on big ranges. Even if we use a better algorithm than sorting the range to find the top 10, the best case is still O(n²) ...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name preComputeCriterion/preComputedCriterion also kinda sucks, I'm open to propositions

Copy link
Contributor

@fdamhaut fdamhaut left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small functional nitpick, I don't have a better idea for the name of the preComputed Value

This commits adds the "top10" conditional formatting operator, which
highlights the top or bottom N values in a selected range.

Task: 5367065
@hokolomopo hokolomopo force-pushed the master-additional-cfs-adrm branch from d41f50a to 7e1c6a3 Compare December 2, 2025 09:40
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.

3 participants