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

Challenge #2: OrdersList Time Format #3

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bchehraz
Copy link
Owner

Changes

List Changes Introduced by this PR

  1. Replaces current method of outputting time via getHours(), getMinutes(), getSeconds() methods to just use toLocaleTimeString(...) instead.

Purpose

Describe the problem or feature in addition to a link to the issues.

Approach

How does this change address the problem?

  • I simply added .toLocaleTimeString('en-US', { hour12: false }) to convert the Date object to hh:mm:ss. It was not specified whether this project was meant to be used for any locale other than en-US, so I made the decision to use en-US and converted to 24-hour time to meet the specified hh:mm:ss format.
  • Alternatively I could have added an npm package like moment.js; however, this would add additional functionality that would exceed the needs of the task and scope of the project OR I could have added a helper function to manually convert the time string to the correct readable format. Using Date.toLocaleTimeString seems to be the best fit here.

Pre-Testing TODOs

What needs to be done before testing

  • Make sure there is at least 1 order stored in the database before testing, with a hours, minutes, or seconds value less than 10.

Testing Steps

How do the users test this change?

  • Simply visit the /view-orders page to see that the time value is displaying correctly.

Learning

Describe the research stage

  • I simply looked up what methods the Date object has and determined toLocaleTimeString would best fit the needs of the task.

Links to blog posts, patterns, libraries or addons used to solve this problem

Closes Shift3#2

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.

Bug: Order Time does not format time correctly
1 participant