Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Timestamp '1970/01/01 00:00:00' formats as 'undefined' in hovertext #6751

Closed
adamjhawley opened this issue Oct 18, 2023 · 2 comments
Closed
Labels
bug something broken

Comments

@adamjhawley
Copy link
Contributor

I tried to customise the hovertext format of some timestamp data and found that the timestamp '1970/01/01 00:00:00' formats as 'undefined'.

Note: please hover over the first data point in the example below to see the issue I am describing.

Minimal example to recreate the issue (CodePen here):

var dates = [0,1].map(x => `1970-01-01 00:00:0${x}`);

var data = [
  {
    type: 'scatter',
    x: dates,
    y: [5,2,3,1,4],
    hovertemplate: "%{x}, %{y}"
  }]

var layout = {
  xaxis: {
    tickformat: '%H:%M:%S',
    hoverformat: '%H:%M:%S'
  },
}

Plotly.newPlot('myDiv', data, layout);

I am happy to try and fix this issue myself but could benefit from someone pointing me in the right direction. I presume that the issue is caused by something like "0 seconds since epoch" is wrongly resolved as falsey at some point.

Thanks in advance for any pointers/help!

@archmoj archmoj added the bug something broken label Oct 18, 2023
@archmoj
Copy link
Contributor

archmoj commented Oct 18, 2023

Nice find 🥇
After long investigation,
it looks like the bug is coming from this block:

// can we remove the whole time part?
if(dateStr === '00:00:00' || dateStr === '00:00') {
dateStr = headStr;
headStr = '';
} else if(dateStr.length === 8) {
// strip off seconds if they're zero (zero fractional seconds
// are already omitted)
// but we never remove minutes and leave just hours
dateStr = dateStr.replace(/:00$/, '');
}

Perhaps we should modify the first if statement to be something like:

if(headStr !== undefined && (dateStr === '00:00:00' || dateStr === '00:00')) {

I am looking forward to reviewing your pull request.
FYI - Our contributing information is at https://github.com/plotly/plotly.js/blob/master/CONTRIBUTING.md

adamjhawley added a commit to adamjhawley/plotly.js that referenced this issue Oct 18, 2023
This patch fixes issue plotly#6751, where the epoch timestamp '1970-01-01 00:00:00',
with 'hoverformat' '%H:%M:%S' is formatted as 'undefined'. This patch fixes the
issue by ensuring that zeroes are only stripped if 'extraPrecision' caused
trailling zeroes in the formatted date.
adamjhawley added a commit to adamjhawley/plotly.js that referenced this issue Oct 18, 2023
This patch fixes issue plotly#6751, where the epoch timestamp '1970-01-01 00:00:00',
with 'hoverformat' '%H:%M:%S' is formatted as 'undefined'.

Signed-off-by: adamjhawley <adamhawley99@gmail.com>
@adamjhawley
Copy link
Contributor Author

Thanks @archmoj for the pointers! I have now created PR #6752.

In addition to your suggestion, I had to add another change which prevents the function from trailing stripping zeroes when they were not added because of 'extraPrecision'.

@archmoj archmoj closed this as completed Oct 20, 2023
adamjhawley added a commit to adamjhawley/plotly.js that referenced this issue Oct 26, 2023
Previously, the formatting of the epoch timestamp using custom a hover format
did not work as expected (plotly#6751).

This patch adds a test to help prevent this issue re-occuring in a regression.

Signed-off-by: adamjhawley <adamhawley99@gmail.com>
adamjhawley added a commit to adamjhawley/plotly.js that referenced this issue Oct 26, 2023
Previously, the formatting of the epoch timestamp using custom a hover format
did not work as expected (plotly#6751).

This patch adds a test to help prevent this issue reappearing in a regression.

Signed-off-by: adamjhawley <adamhawley99@gmail.com>
adamjhawley added a commit to adamjhawley/plotly.js that referenced this issue Oct 26, 2023
Previously, the formatting of the epoch timestamp using custom a hover format
did not work as expected (plotly#6751).

This patch adds a test to help prevent this issue reappearing in a regression.

Signed-off-by: adamjhawley <adamhawley99@gmail.com>
adamjhawley added a commit to adamjhawley/plotly.js that referenced this issue Oct 26, 2023
Previously, the formatting of the epoch timestamp using custom a hover format
did not work as expected (plotly#6751).

This patch adds a test to help prevent this issue reappearing in a regression.

Signed-off-by: adamjhawley <adamhawley99@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something broken
Projects
None yet
Development

No branches or pull requests

2 participants