[Bugfix] Time Formatting #2145
[Bugfix] Time Formatting #2145
Conversation
src/common/helpers.ts
Outdated
| @@ -64,6 +65,22 @@ export function date(date: number | string, format: string) { | |||
| return dayjs(date).format(format); | |||
| } | |||
|
|
|||
| export function useFormatTime() { | |||
There was a problem hiding this comment.
Isn't this a duplicate of useDateTime?
There was a problem hiding this comment.
@beganovich Actually, it's not a duplicate because useFormatTime is dedicated only to formatting time values, while useDateTime formats the full date + time. However, since you mentioned it, I believe it's better to have it under useDateTime so we can make it more global. So, I've modified the useDateTime hook to accept time-only formatting. Let me know your thoughts.
| } | ||
|
|
||
| return dayjs | ||
| .utc(date) | ||
| .tz(timeZone || companyTimeZone) |
There was a problem hiding this comment.
@beganovich After our quick discussion in the group about time zones, I'm now aware that we don't need to add timezone here. So, I've removed it.
There was a problem hiding this comment.
But this hook is used in different places? Is the behavior same after your change?
There was a problem hiding this comment.
@beganovich Currently, the hook is only used in three places for formatting the next_send_datetime column value in Recurring Invoices. While we have many similar column formatting instances throughout the tables, this particular one formats both date and time.
The behavior will change as it will no longer use the company timezone. However, this aligns with our existing formatting logic, as we don't use company timezones for any other date or time column formatting in the application. The dayjs will take the timezone from the browser and format it into that timezone, which is how we handle all other formatting.
Basically change aligns logic with the rest of the app.
@beganovich @turbo124 The PR includes formatting of times in the app when we display them. Wherever I found a fixed time format, I replaced it with a dynamic one relying on the
military_timecompany settings property. Screenshot:Let me know your thoughts.
Closes #2142