Add a "Sample timestamp" field to the sample tooltip in timeline

[canova]

Fixes #5319.

This also came up from a review comment here: #5298 (comment)

It sounds like a good idea to add the sample time to the sample tooltips. So 1) it will be more explicit that this is indeed a single sample. 2) It could be useful to know the precise time of a sample, so we can potentially compare different samples.

Apparently this component wasn't localized. So I filed #5321 for that. I'm not so sure if we should really localize it right now, because I think last time we discussed, we were saying that if we have <label>: <value> data in a view and value itself is not localized, maybe we shouldn't localize the label either. But I'm open to suggestions. I think this can be done outside of this PR.

Deploy preview / production

Example screenshot:

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.08%. Comparing base (9175e24) to head (b49933a).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #5322   +/-   ##
=======================================
  Coverage   86.08%   86.08%           
=======================================
  Files         311      311           
  Lines       29678    29683    +5     
  Branches     8190     8192    +2     
=======================================
+ Hits        25548    25553    +5     
  Misses       3548     3548           
  Partials      582      582           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[mstange]

I worry that "Sample time" is ambiguous and might lead people to believe that it means "sample duration".

How do you feel about these alternatives?

• Sample timestamp: <time>
• Sample collected at <time>
• Sampled at <time>

[canova]

I worry that "Sample time" is ambiguous and might lead people to believe that it means "sample duration".

How do you feel about these alternatives?

* `Sample timestamp: <time>`

* `Sample collected at <time>`

* `Sampled at <time>`

Yeah, you're right about it being ambiguous. I think I would prefer Sample timestamp: <time> out of these 3, to keep this <label>: <value> unanimity in the tooltip. Changed the PR to reflect that. Thanks for the feedback!

[julienw]

I have a different suggestion (sorry!).
I'm thinking of something that could be reused in the marker tooltip, so that it would be consistent across the UI.

I have this niche prior art in subsurface, an open source diving program:

The "@" part is the timestamp of the dive that my mouse is currently hovering.

I think that simply @ is not that clear. But what about At: ?

[canova]

I have a different suggestion (sorry!).

I'm thinking of something that could be reused in the marker tooltip, so that it would be consistent across the UI.

I have this niche prior art in subsurface, an open source diving program:

The "@" part is the timestamp of the dive that my mouse is currently hovering.

I think that simply @ is not that clear. But what about At: ?

Sharing things between components is probably fine. But the reason @fqueze suggested adding this was to make it even more obvious "what" this tooltip belonged to. This way people will see "sample" at first and won't have to think what tooltip this is. I don't think removing "sample" and only leaving "at" solves this problem.

[julienw]

Ah ok I see, it makes sense too.

Then I think I'd mildly prefer Sample at: (with the semicolon) because it's smaller, but I'll leave it up to you to decide.

[julienw]

thanks!

Now that we have this timestamp in the tooltip, it's pretty clear that sometimes the sample tooltip's timestamp and the selection's timestamp don't agree completely, see below:

Some hit testing logic may be buggy or just not the same, but it would be good to understand eventually...

[canova]

Thanks, for the review. I merged your and Markus' suggestions and renamed it to Sampled at:.

Now that we have this timestamp in the tooltip, it's pretty clear that sometimes the sample tooltip's timestamp and the selection's timestamp don't agree completely, see below:

Some hit testing logic may be buggy or just not the same, but it would be good to understand eventually...

Hm, there could be multiple samples in a pixel and the way we choose these hovered samples could be different. But it's worth taking a look.

[julienw]

Hm, there could be multiple samples in a pixel and the way we choose these hovered samples could be different. But it's worth taking a look.

yeah that might be just that. In the screenshot the different might look big but it may not be that big depending on the zoom level (I didn't do the math).