Time unit fixes

[will-moore]

Fixes #385

This PR handles time values up to Days (previously, anything over 99 hours was not displayed correctly). It also handles any time units.

Tested using script at https://github.com/ome/omero-iviewer/issues/385#issuecomment-874824752 to set units for existing image.

When doing this for a sample (FRAP) image, timestamps that are less than a day, no 'days' are shown:

But when the time is longer than a day it is show like this (NB: open to suggestions for best way to format days)?

If the time is an exact number of days, e.g. 3 days, we still show hh:mm:ss.sss. Is it worth eliminating that if all those values are zero?

cc @jburel

[jburel]

If the time is an exact number of days, e.g. 3 days, we still show hh:mm:ss.sss. Is it worth eliminating that if all those values are zero?

My vote is yes Also the precision ss.sss when hours or even minutes are displayed seems too much From your screenshot it looks like the time is a negative value when no days are displayed

[will-moore]

Hmmm - I'm not sure I agree. When my digital clock says 11 o'clock, it's 11:00 not 11. Same for a stop-watch. The display doesn't change just because you hit a round number.

If move the T-slider from 1 day to 1 day 00:00:00.001 then there's going to be a big shift in the length of the displayed time string, causing the slider to contract to provide more space.

If a user sees time displayed as 1 day they won't know whether that's really "1 day 2 hours" or "1 day 1 second" rounded to 1 day, or if it's precisely 1 day. Showing the full accuracy and being consistent across all time-points is much clearer.

If we take a more advanced strategy later, and we display times differently depending on the units on the server, then we could consider some other behaviours (as well as showing the server-side units), but that is a whole different change.

The negative values in that screenshot are correct. It's a FRAP movie where frames before the bleach event have negative timestamps.

I ran the script https://github.com/ome/omero-iviewer/issues/385#issuecomment-874824752 on this FRAP Image (user-3): https://merge-ci.openmicroscopy.org/web/webclient/img_detail/83221/ and this DV: https://merge-ci.openmicroscopy.org/web/webclient/?show=image-122915

[will-moore]

An example from IDR where this will affect the formatting: https://idr.openmicroscopy.org/webclient/img_detail/10340802/

96:00:00.000 will be 4 days 00:00:00.000.

[sbesson]

Following up on the comment from https://github.com/ome/omero-iviewer/pull/386#issuecomment-878124142 and this morning's discussion, my biggest concern is about the custom nature of the proposed format. For usability, I agree it makes complete sense to convert the information into the most relevant unit i.e. 4 days is clearer and more explicit than 96h. It also has the benefit of keeping the time suffix compatible with ISO 8601 where the hour is bounded between 00 and 23.

My main issue is that the proposed representation mixes unit written as words and unit separators which I found confusing. I would have almost expect something 4 days 4 hours 2 minutes 10 seconds. Otherwise, would a truncated version of the ISO standard i.e. dd hh:mm:ss[.mmm] be an option?

[jburel]

4 days 4 hours 2 minutes 10 seconds will be too long to fit in the available space. So I think the suggested truncated version of the ISO standard is probably the most appropriate one

[will-moore]

I've fixed the formatting, adding a tooltip to improve clarity:

Needed to change a bit more code, so please test values shown while sliding, beside the slider AND tooltip.

[jburel]

Looking at https://merge-ci.openmicroscopy.org/web/webclient/img_detail/83221/ If possible we could be a be smarter in the tooltip 1 days always makes me jump

[will-moore]

@jburel Done

[jburel]

I have installed this PR in pilot-0113 The time is now displayed in days instead of seconds.

[jburel]

The tooltip still displays 0hours but this can be fixed in a follow-up PR

[jburel]

https://idr.openmicroscopy.org/webclient/img_detail/10340802/

idr

pilot with this PR

[jburel]

suggest to merge it and release as 0.11.1

[jburel]

Merging and tagging