Update/formattime #101
Update/formattime #101
Conversation
capetillo
requested review from
mgdaily,
jnation3406 and
LTDakin
and removed request for
mgdaily,
jnation3406 and
LTDakin
| const date = new Date(timeString) | ||
| const options = { hour: 'numeric', minute: 'numeric' } | ||
| return date.toLocaleTimeString('en-US', options) | ||
| if (options && (options.hour || options.minute)) { |
There was a problem hiding this comment.
what is the reasoning for wrapping the date's util functions instead of using them on their own? It might be simpler to just call them in the code, but if you had a reason for doing so then this code looks good
There was a problem hiding this comment.
it was the same function 3 times that formatted time in 3 different way so now it just takes the 2 values as params: the date string and options (i.e. how we want to format the time)
| const date = new Date(timeString) | ||
| const options = { hour: 'numeric', minute: 'numeric' } | ||
| return date.toLocaleTimeString('en-US', options) | ||
| if (options && (options.hour || options.minute)) { |
There was a problem hiding this comment.
can you comment this to say why we need this conditional statement, or what the correct format of options is? might help a future programmer. alternatively, you can expose the specific options keys as function parameters.
There was a problem hiding this comment.
yes I can do that!
Updates utils functions
formatDate,formatTime, andformatDateMMDDYYinto one:formatDateTimeto further simplify the utility functionality.Updates tests too
VISUALS
Date and time rendering as it should
