Fix timetable previous stop time checks from checking text columns. #772
Conversation
…rom checking text columns.
miles-grant-ibigroup
left a comment
miles-grant-ibigroup
left a comment
There was a problem hiding this comment.
I can't believe this was the fix... well done figuring it out!
.gitignore
Outdated
| scripts/*client.json | ||
|
|
||
| # Vs code settings | ||
| .vscode |
There was a problem hiding this comment.
Super nit I'd add a trailing slash here to make it clear it's a folder!
There was a problem hiding this comment.
Super nit could be a great marvel character, you may want to trademark that
binh-dam-ibigroup
left a comment
binh-dam-ibigroup
left a comment
There was a problem hiding this comment.
It would be nice of the validation ignored the first few columns instead, but as written it is fine.
lib/editor/selectors/timetable.js
Outdated
| let previousIndex = colIndex - 1 | ||
| // Find previous stop time | ||
| while (previousValue === null && previousIndex >= 0) { | ||
| while (previousValue === null && previousIndex >= 0 && columns[previousIndex].type !== 'TEXT') { |
There was a problem hiding this comment.
Wondering whether we should check whether the previous column index is the last column index for non-stop-time trip data?
There was a problem hiding this comment.
That's a good idea, I changed the condition to only be if the type is ARRIVAL_TIME or DEPARTURE_TIME in 896ebe0
There was a problem hiding this comment.
if you'd prefer I guess we could find the index of the first stop time column and check based on that, but this seems simpler
There was a problem hiding this comment.
What you did using ARRIVAL_TIME or DEPARTURE_TIME is good too!
…s (more strict check)
philip-cline
assigned
binh-dam-ibigroup and unassigned
binh-dam-ibigroup and
philip-cline
Checklist
devbefore they can be merged tomaster)Description
Currently, naming a trip ID (or filling any other text field) with a number greater than the start time of the first arrival time produces a validation error in the UI since validation takes place for all columns left of the stop time. This PR adds a condition to only check columns that are not of type 'TEXT'.
The PR also adds VS code
settings.jsonto the.gitignorefile, which I hope won't be controversial.Issue screenshot:
