[IMP] pivot: sort in dashboard/read-only mode #5654

LucasLefevre · 2025-02-06T12:27:40Z

Description:

description of this task, what is implemented and why it is implemented that way.

Task: TASK_ID

review checklist

robodoo · 2025-02-06T12:27:43Z

LucasLefevre · 2025-02-10T13:30:12Z

robodoo help

robodoo · 2025-02-10T13:30:15Z

Currently available commands for @LucasLefevre:

command
`help`	displays this help
`r(eview)+`	approves the PR, if it's a forwardport also approves all non-detached parents
`r(eview)=<number>`	only approves the specified parents
`fw=no`	does not forward-port this PR
`fw=default`	forward-ports this PR normally
`fw=skipci`	does not wait for a forward-port's statuses to succeed before creating the next one
`fw=skipmerge`	does not wait for the source to be merged before creating forward ports
`up to <branch>`	only ports this PR forward to the specified branch (included)
`merge`	integrate the PR with a simple merge commit, using the PR description as message
`rebase-merge`	rebases the PR on top of the target branch the integrates with a merge commit, using the PR description as message
`rebase-ff`	rebases the PR on top of the target branch, then fast-forwards
`squash`	squashes the PR as a single commit on the target branch, using the PR description as message
`delegate+`	grants approval rights to the PR author
`delegate=<...>`	grants approval rights on this PR to the specified github users
`default`	stages the PR normally
`priority`	tries to stage this PR first, then adds `default` PRs if the staging has room
`alone`	stages this PR only with other PRs of the same priority
`cancel=staging`	automatically cancels the current staging when this PR becomes ready
`override=<...>`	marks overridable statuses as successful
`check`	fetches or refreshes PR metadata, resets mergebot state

Note: this help text is dynamic and will change with the state of the PR.

hokolomopo

The sky is blue,
The herb is green,
And the runbot is red

Didn't read the code yet, only functional notes

hokolomopo · 2025-06-02T06:38:50Z

src/actions/action.ts

@@ -36,7 +36,7 @@ export interface ActionSpec {
   * Can be defined to display an icon


Hovering an empty pivot cell does nothing. That's a bit of a problem, because it closes the sorting menu when after sorting the hovered cell becomes empty

src/actions/action.ts

hokolomopo · 2025-06-02T06:44:18Z

src/actions/action.ts

@@ -36,7 +36,7 @@ export interface ActionSpec {
   * Can be defined to display an icon
   */


Disco party ! 🪩

https://drive.google.com/file/d/1siaXpupi-ux4V3EbPf32jenpsH6kw-86/view?usp=drive_link

Highlight should be when menu is open, not when it's hovered

hokolomopo · 2025-06-02T06:45:47Z

src/actions/action.ts

@@ -36,7 +36,7 @@ export interface ActionSpec {
   * Can be defined to display an icon
   */


Menu opening feels a bit sluggish, hover delay seems a bit long (why a delay at all?)

hokolomopo · 2025-06-02T06:47:40Z

src/actions/action.ts

@@ -36,7 +36,7 @@ export interface ActionSpec {
   * Can be defined to display an icon
   */


Is it just me, or is the top/bottom padding of the menu smaller than the one of the other menus ?

hokolomopo · 2025-06-02T06:51:02Z

src/actions/action.ts

@@ -36,7 +36,7 @@ export interface ActionSpec {
   * Can be defined to display an icon
   */


Well I'm colorblind, but the "sorting is active" color in the menu is not obvious to me 😔 Was the check mark we use for "active" menu elements in other menus ugly ?

hokolomopo · 2025-06-02T06:54:54Z

src/actions/action.ts

@@ -36,7 +36,7 @@ export interface ActionSpec {
   * Can be defined to display an icon
   */


wrong item is hovered when menu appear

https://drive.google.com/file/d/1IHqnpZXeowx_1a8vbAb56waPSJ-DHSAk/view?usp=drive_link

The props is never used and `getPopoverContainerRect` is not always present in the env Task: 4543812

Steps to reproduce: - create a dashboard with a small amount of columns (less the the width of your screen, just like standard dashboard) - on the right-most column, insert a formula with an error (=0/0) - in dashboard mode, hover the error cell => the error message popover is display on the left of the cell, even though there's room on the right. It's not critical for those error messages, but a following commit adds a popover widget to sort pivot columns. Task: 4543812

Extract the menu items to its own component. The goal is to decouple it from the popover and positioning logic. Task: 4543812

The current menu hovering mechanism only works well for menu items generated at runtime, but not for statically declated actions. Let's say I want to declare a menu items which highlights a zone when hovered. When unregistering, I don't have the reference to the highlight provider because it's not global ```ts myRegistry.add("my-menu", { ... // register the highlight onStartHover(env) { const highlightProvider = { get highlights() { return getPivotHighlights(env.model.getters); }, }; env.getStore(HighlightStore).register(highlightProvider) }, // clean the highlight onStopHover: { // you would need to do this, but you don't have any reference to // `highlightProvider` at this point env.getStore(HighlightStore).unRegister(highlightProvider) }, }); ```

Task: 4543812

This commit adds the abitlity to add a transition when the popover moves from one position to another Task: 4543812

In our dashboards, the pattern "Top 10 XXXX" is used a lot. Let's say you have the top 10 countries with the most revenue. But what are the worst ones ? Or what are the countries with the most/least number of leads ? This task add a way to sort pivot measures. A little tooltip appears when hovering a pivot cell. Task: 4543812

Since the last commit, some core commands (UPDATE_PIVOT for now) can be dispatched in read-only/dashboard mode. But those commands should remain local, should not be registered to the database and not sent to other clients. Only the client related messages (client joined, left or moved) should be sent to other clients. This commit adds a wrapper around the transport service to block all other messages. Task: 4543812

Currently, when hovering a cell with an error, a link, or pivot sorting tooltip, the user have to wait for 300ms before the popover is displayed. It feels slugish (Gsheet is much faster) but most importantly, it makes it hard to discover the pivot sorting feature because there's no hint that the cell is hoverable and it's not natural to stop on a cell for a full 300ms for no apparent reason. Task: 4543812

LucasLefevre changed the title ~~Master sort pivot tooltip lul~~ [IMP] pivot: sort in dashboard/read-only mode Feb 6, 2025

LucasLefevre force-pushed the master-sort-pivot-tooltip-lul branch from 800b859 to c37754d Compare February 6, 2025 12:34

LucasLefevre force-pushed the master-sort-pivot-tooltip-lul branch 3 times, most recently from b72e30e to 30dfefe Compare February 18, 2025 14:38

LucasLefevre force-pushed the master-sort-pivot-tooltip-lul branch from 30dfefe to 3aed9fb Compare February 28, 2025 12:34

LucasLefevre force-pushed the master-sort-pivot-tooltip-lul branch 2 times, most recently from b148bca to dfdba01 Compare April 23, 2025 14:03

LucasLefevre force-pushed the master-sort-pivot-tooltip-lul branch 9 times, most recently from e5268bd to 32178d9 Compare May 9, 2025 12:45

LucasLefevre force-pushed the master-sort-pivot-tooltip-lul branch 5 times, most recently from efdffb1 to 8995615 Compare May 13, 2025 13:01

hokolomopo reviewed Jun 2, 2025

View reviewed changes

LucasLefevre added 4 commits June 2, 2025 12:55

[REF] popover: remove unused props

1b15ec4

The props is never used and `getPopoverContainerRect` is not always present in the env Task: 4543812

[REF] menu: extract menu items

6d8e00c

Extract the menu items to its own component. The goal is to decouple it from the popover and positioning logic. Task: 4543812

LucasLefevre added 5 commits June 2, 2025 12:55

[IMP] menu: dynamic icon color

8f946fa

Task: 4543812

[IMP] popover: add optional transition

e1525c0

This commit adds the abitlity to add a transition when the popover moves from one position to another Task: 4543812

LucasLefevre force-pushed the master-sort-pivot-tooltip-lul branch from 8995615 to af0602d Compare June 2, 2025 10:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[IMP] pivot: sort in dashboard/read-only mode #5654

[IMP] pivot: sort in dashboard/read-only mode #5654

Uh oh!

LucasLefevre commented Feb 6, 2025

Uh oh!

robodoo commented Feb 6, 2025

Uh oh!

LucasLefevre commented Feb 10, 2025

Uh oh!

robodoo commented Feb 10, 2025

Uh oh!

hokolomopo left a comment

Uh oh!

hokolomopo Jun 2, 2025

Uh oh!

Uh oh!

hokolomopo Jun 2, 2025

Uh oh!

hokolomopo Jun 2, 2025

Uh oh!

hokolomopo Jun 2, 2025

Uh oh!

hokolomopo Jun 2, 2025

Uh oh!

hokolomopo Jun 2, 2025

Uh oh!

Uh oh!

		@@ -36,7 +36,7 @@ export interface ActionSpec {
		* Can be defined to display an icon

[IMP] pivot: sort in dashboard/read-only mode #5654

Are you sure you want to change the base?

[IMP] pivot: sort in dashboard/read-only mode #5654

Uh oh!

Conversation

LucasLefevre commented Feb 6, 2025

Description:

review checklist

Uh oh!

robodoo commented Feb 6, 2025

Uh oh!

LucasLefevre commented Feb 10, 2025

Uh oh!

robodoo commented Feb 10, 2025

Uh oh!

hokolomopo left a comment

Choose a reason for hiding this comment

Uh oh!

hokolomopo Jun 2, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hokolomopo Jun 2, 2025

Choose a reason for hiding this comment

Uh oh!

hokolomopo Jun 2, 2025

Choose a reason for hiding this comment

Uh oh!

hokolomopo Jun 2, 2025

Choose a reason for hiding this comment

Uh oh!

hokolomopo Jun 2, 2025

Choose a reason for hiding this comment

Uh oh!

hokolomopo Jun 2, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!