Fix hover with period alignment points and improve positioning of spikes and unified hover label #5846

archmoj · 2021-07-21T14:48:29Z

Fixes #5822, fixes #5841 and fixes #5721.

@plotly/plotly_js

nicolaskruchten · 2021-07-21T15:31:03Z

This looks good AFAICT! @alexcjohnson can we get an expedited review plz?

alexcjohnson · 2021-07-21T16:14:02Z

test/jasmine/tests/hover_label_test.js

+                    'bar : 1'
+                ]});
+
+                _hover(gd, { xpx: 210, ypx: 200 });
                assertLabel({title: 'Jan', items: [
                    'bar : (Jan 1, 2000, 1)',


Line 5348 has a typo that's making this test confusing (xhoverfrmat)

Good eye. Addressed in 7c37e98.

alexcjohnson · 2021-07-21T16:38:25Z

Looking specifically at the test I commented on above but having fixed the typo, here's the behavior on master:

It's not perfect, I don't think we should be showing Jan while the cursor is over the first half of Feb, but otherwise I actually think it looks correct: the spikeline shows the point that wins and the hover label shows all traces that match that period and it's positioned at one edge of the period.

Here's the behavior currently with this PR:

Now the hoverlabel is positioned at the spikeline regardless of the period, it very often does NOT contain all the traces with points in a matching period, and when you do get multiple traces you mix points representing different periods - with some very weird flipping between two winning points right around the period boundary.

I thought at one point we had the idea that a period-positioned scatter trace should behave like a bar for hover purposes, in that any given point should be included in the hover whenever your cursor is over the period it represents. Did we drop that idea?

nicolaskruchten · 2021-07-21T16:41:13Z

a period-positioned scatter trace should behave like a bar for hover purposes

Yes, here are the period-positioning invariants: #5553

The behaviour you note above is indeed troubling. Which mock is this?

nicolaskruchten · 2021-07-21T16:43:26Z

I'm also seeing some odd spikeline/hover mismatches in the all-bar case: https://codepen.io/nicolaskruchten/pen/bGWoVKB?editors=0010

alexcjohnson · 2021-07-21T16:44:09Z

It's this, from the test I commented on:

var scatterType = 'scatter';
Plotly.newPlot(gd, {
                data: [
                    {
                        name: 'bar',
                        type: 'bar',
                        x: ['2000-01', '2000-02'],
                        y: [1, 2],
                        xhoverformat: '%b',
                        xperiod: 'M1'
                    },
                    {
                        name: 'start',
                        type: scatterType,
                        x: ['2000-01', '2000-02'],
                        y: [1, 2],
                        xhoverformat: '%b',
                        xperiod: 'M1',
                        xperiodalignment: 'start'
                    },
                    {
                        name: 'end',
                        type: scatterType,
                        x: ['2000-01', '2000-02'],
                        y: [1, 2],
                        xhoverformat: '%b',
                        xperiod: 'M1',
                        xperiodalignment: 'end'
                    },
                ],
                layout: {
                    showlegend: false,
                    width: 600,
                    height: 400,
                    hovermode: 'x unified'
                }
            })

alexcjohnson · 2021-07-21T21:11:02Z

Better, it's now consistently showing all the traces I think it should. I still think we need to push the hover label outside the period the winning point represents, and right now it seems the hover label and spike line positions are referenced to different points: the label at the first (?) point in the hover set, but the spike line at the winning point. For example in the period_positioning3 mock, with my cursor near the end of May, I can't see the brown point or most of the red point being hovered on, and the spike line is obscured:

nicolaskruchten · 2021-07-21T21:18:30Z

Agreed with Alex's point above: the hoverlabel should always be to the side of the spikeline (as it is on master I believe)

…inning point

… with data points

nicolaskruchten · 2021-07-22T15:33:55Z

With this build, I don't seem to be able to hover on the left-most points in the figure from #5822 ... see https://codepen.io/nicolaskruchten/pen/MWmEzeJ?editors=0010

archmoj · 2021-07-22T15:36:49Z

With this build, I don't seem to be able to hover on the left-most points in the figure from #5822 ... see https://codepen.io/nicolaskruchten/pen/MWmEzeJ?editors=0010

Good catch. Thank you!

archmoj · 2021-07-22T20:17:22Z

But I can see your argument, I'd be OK with pushing the label back to the spike line at least when it's a scatter point that wins. I think if it's a bar that wins it's still better to push it to the edge of the bar, and in fact to the edge of the bar group when we're labeling the whole group.

@archmoj can we do this please? When the scatter point wins, it's ok for the hovelabel to occlude the bar.

Good idea. Applied in 16f4554.

alexcjohnson · 2021-07-22T21:18:14Z

It's still possible to have the hover label occluding the spikeline - hover on the right half of any period in period_positioning3

nicolaskruchten · 2021-07-22T21:59:39Z

I think this is a case of the spike line not matching the hover-winning point, which is indeed a bug, and it seems worse with this PR than on master. Can we see if there's a fix for it plz?

archmoj · 2021-07-22T22:29:56Z

It's still possible to have the hover label occluding the spikeline - hover on the right half of any period in period_positioning3

That's simply because of "When the scatter point wins, it's ok for the hovelabel to occlude the bar" logic added in 16f4554. Without it there would be no occlusion with spikeline :)

archmoj · 2021-07-22T22:32:14Z

I think this is a case of the spike line not matching the hover-winning point, which is indeed a bug, and it seems worse with this PR than on master. Can we see if there's a fix for it plz?

This is also true : )

nicolaskruchten · 2021-07-22T22:34:53Z

Let's try to fix the spike line position too please before merging this. It should not be possible for the spike line to be on a different point that the winning point: there's a specific attribute for that.

archmoj · 2021-07-22T22:36:04Z

Let's try to fix the spike line position too please before merging this. It should not be possible for the spike line to be on a different point that the winning point: there's a specific attribute for that.

On it.

archmoj · 2021-07-22T23:38:09Z

Let's try to fix the spike line position too please before merging this. It should not be possible for the spike line to be on a different point that the winning point: there's a specific attribute for that.

Done in dca5b1e.

- also adjust scattergl and splom hover label position

nicolaskruchten · 2021-07-23T01:50:47Z

Closer! So in this pen: https://codepen.io/nicolaskruchten/pen/xxdPKwZ?editors=0010 when I hover over the left-most point I get no bar in the hoverlabel, and when I hover on the next point I get the left-most bar in the hoverset. This is even more apparent when hovermode = 'x'.

I should add that the vertical position over the hoverlabels seems too low in this pen: https://codepen.io/nicolaskruchten/pen/QWvOLjJ?editors=0010 ... the hoverlabel is always around the bottom two points for me.

archmoj · 2021-07-23T12:51:17Z

I should add that the vertical position over the hoverlabels seems too low in this pen: https://codepen.io/nicolaskruchten/pen/QWvOLjJ?editors=0010 ... the hoverlabel is always around the bottom two points for me.

Good catch. Fixed in df6353d

archmoj · 2021-07-23T12:51:43Z

Closer! So in this pen: https://codepen.io/nicolaskruchten/pen/xxdPKwZ?editors=0010 when I hover over the left-most point I get no bar in the hoverlabel, and when I hover on the next point I get the left-most bar in the hoverset. This is even more apparent when hovermode = 'x'.

That's a bug.

archmoj · 2021-07-23T13:53:32Z

Closer! So in this pen: https://codepen.io/nicolaskruchten/pen/xxdPKwZ?editors=0010 when I hover over the left-most point I get no bar in the hoverlabel, and when I hover on the next point I get the left-most bar in the hoverset. This is even more apparent when hovermode = 'x'.

That's a bug.

Addressed in 9217db4.

nicolaskruchten · 2021-07-23T14:48:05Z

OK, this last set of changes nails down everything I care about! There's still some oddity around end-positioned grouped-bars but we can leave that out for now.

@alexcjohnson over to you, again :)

…t fit in subplot

…riod_positioning9 mock & horizontal variant

alexcjohnson

💃 I think we have a winner! 🏆

archmoj added 4 commits July 21, 2021 10:40

fix scattergl test

b4bdec3

fix hover on scatter and scattergl with periods

53fea8e

fix hover on bars with period

2933803

improve and adjust jasmine tests

775a8e0

archmoj added bug something broken status: reviewable labels Jul 21, 2021

archmoj added this to the v2.3.0 milestone Jul 21, 2021

archmoj requested review from alexcjohnson and nicolaskruchten July 21, 2021 14:51

draft log for PR 5846

168bc75

drop unused line

3deca0d

alexcjohnson reviewed Jul 21, 2021

View reviewed changes

fix typo and adjust test

7c37e98

scatters with period ranges

e5a4057

archmoj requested a review from alexcjohnson July 21, 2021 20:12

archmoj added 2 commits July 21, 2021 16:15

adjust scatter and bar test titles

bd528a2

fit -> it

85821d1

archmoj added 3 commits July 22, 2021 08:38

refactor: enter long lines

9fbd186

position hover labels in respect to max and min of hoverset not the w…

8a36d8f

…inning point

for end alignment of unified hover label use minimum to avoid overlap…

f128cf9

… with data points

similar positioning of hover box for y unified

b26fcbb

archmoj added 2 commits July 22, 2021 14:30

edit PR log

c62d520

simplify logic and handle the case of not fitting inside the subplot

21a1a93

archmoj added 3 commits July 22, 2021 16:20

when the scatter point wins, OK to occlude bar & other points

16f4554

fix syntax

8ed847a

drop out of range pixel from the end

30f61c8

for x|y hovermodes display spikeline on the winning point

dca5b1e

- also adjust scattergl and splom hover label position

adjust vertical and horizontal alignment of unified hover label

df6353d

archmoj added 2 commits July 23, 2021 09:55

handle end edge case

9217db4

adjustment also for winning types e.g. heatmap use scatter logic

1c7a69d

choose the side of the paper which is closest if unified hover did no…

f0ef7db

…t fit in subplot

archmoj changed the title ~~Fix misplaced hover with period positioning~~ Fix hover with period alignment points and improve positioning of spikes and unified hover label Jul 23, 2021

archmoj added 2 commits July 23, 2021 10:58

update PR log

4a0a504

move unified hover by one pixel better help adjustment tested with pe…

cd0b469

…riod_positioning9 mock & horizontal variant

alexcjohnson approved these changes Jul 23, 2021

View reviewed changes

archmoj merged commit 87e4e12 into master Jul 23, 2021

archmoj deleted the fix5822-hover-with-period branch July 23, 2021 16:15

archmoj mentioned this pull request Aug 30, 2021

Handle case of div with zero dimension when positioning unified hover box #5913

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix hover with period alignment points and improve positioning of spikes and unified hover label #5846

Fix hover with period alignment points and improve positioning of spikes and unified hover label #5846

archmoj commented Jul 21, 2021 •

edited

Loading

nicolaskruchten commented Jul 21, 2021

alexcjohnson Jul 21, 2021

archmoj Jul 21, 2021

alexcjohnson commented Jul 21, 2021

nicolaskruchten commented Jul 21, 2021

nicolaskruchten commented Jul 21, 2021

alexcjohnson commented Jul 21, 2021

alexcjohnson commented Jul 21, 2021

nicolaskruchten commented Jul 21, 2021

nicolaskruchten commented Jul 22, 2021 •

edited

Loading

archmoj commented Jul 22, 2021

archmoj commented Jul 22, 2021

alexcjohnson commented Jul 22, 2021

nicolaskruchten commented Jul 22, 2021

archmoj commented Jul 22, 2021

archmoj commented Jul 22, 2021

nicolaskruchten commented Jul 22, 2021

archmoj commented Jul 22, 2021

archmoj commented Jul 22, 2021

nicolaskruchten commented Jul 23, 2021

archmoj commented Jul 23, 2021

archmoj commented Jul 23, 2021

archmoj commented Jul 23, 2021

nicolaskruchten commented Jul 23, 2021

alexcjohnson left a comment

Fix hover with period alignment points and improve positioning of spikes and unified hover label #5846

Fix hover with period alignment points and improve positioning of spikes and unified hover label #5846

Conversation

archmoj commented Jul 21, 2021 • edited Loading

nicolaskruchten commented Jul 21, 2021

alexcjohnson Jul 21, 2021

Choose a reason for hiding this comment

archmoj Jul 21, 2021

Choose a reason for hiding this comment

alexcjohnson commented Jul 21, 2021

nicolaskruchten commented Jul 21, 2021

nicolaskruchten commented Jul 21, 2021

alexcjohnson commented Jul 21, 2021

alexcjohnson commented Jul 21, 2021

nicolaskruchten commented Jul 21, 2021

nicolaskruchten commented Jul 22, 2021 • edited Loading

archmoj commented Jul 22, 2021

archmoj commented Jul 22, 2021

alexcjohnson commented Jul 22, 2021

nicolaskruchten commented Jul 22, 2021

archmoj commented Jul 22, 2021

archmoj commented Jul 22, 2021

nicolaskruchten commented Jul 22, 2021

archmoj commented Jul 22, 2021

archmoj commented Jul 22, 2021

nicolaskruchten commented Jul 23, 2021

archmoj commented Jul 23, 2021

archmoj commented Jul 23, 2021

archmoj commented Jul 23, 2021

nicolaskruchten commented Jul 23, 2021

alexcjohnson left a comment

Choose a reason for hiding this comment

archmoj commented Jul 21, 2021 •

edited

Loading

nicolaskruchten commented Jul 22, 2021 •

edited

Loading