Skip to content

Check if the date range is already parsed#42

Closed
noxer wants to merge 1 commit intoflrs:masterfrom
noxer:master
Closed

Check if the date range is already parsed#42
noxer wants to merge 1 commit intoflrs:masterfrom
noxer:master

Conversation

@noxer
Copy link
Copy Markdown

@noxer noxer commented Jan 8, 2020

I've encountered an error when attempting to use updateGraph. The visavail tries to match the Date objects it parsed in the first generate call with the regexes. This PR adds a type check to prevent this.

Caveat: I was unable to generate the minified version due to my enviroment here.

@tanganellilore
Copy link
Copy Markdown
Collaborator

Hi @noxer ,
can you replicate this error on a codesandbox.io ?

I'm not 100% sure that your workaround is correct because when we use the display_date_range we need to be sure that format of time is in the correct form, as same form generated by graph.

@noxer
Copy link
Copy Markdown
Author

noxer commented Jan 9, 2020

Without the fix I'm getting this exception when calling updateGraph:

Uncaught Error: Option of Display range Date/time format not recognized. Pick between 'YYYY-MM-DD' or 'YYYY-MM-DD HH:MM:SS'.
    at HTMLParagraphElement.drawGraph (visavail.js:458)
    at Selection.selection_each [as each] (d3.min.js:1176)
    at chart (visavail.js:278)
    at Selection.selection_call [as call] (d3.min.js:1140)
    at Function.chart.createGraph (visavail.js:1475)
    at Function.chart.updateGraph (visavail.js:1464)
    at updateMeasure (index.html:160)
    at HTMLInputElement.onchange (index.html:59)

The code still checks for the correct format of the timestamps, it just skips that check if the timestamp has already been converted into a Date object.

tanganellilore added a commit that referenced this pull request Jan 10, 2020
@tanganellilore
Copy link
Copy Markdown
Collaborator

Close this merge with new release of visavail 1.3.0.

Enjoy

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants