-
Notifications
You must be signed in to change notification settings - Fork 304
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
HPCC-29403 Allow disabling time zone adjustments in ECLWatch #17655
base: candidate-9.2.x
Are you sure you want to change the base?
Conversation
https://track.hpccsystems.com/browse/HPCC-29403 |
@jeclrsg having some trouble with const currentTime = uiState.isUTC ? Utility.convertToUTCTime(WsDfu.DFUInfo.Modified) : Utility.convertToLocalTime(WsDfu.DFUInfo.Modified); Can you advise... then i can apply to the other pages this is needed on. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kunalaswani Added an example that should fix the error you were getting w/ the time conversion formatter
export function convertToLocalTime(dateString) { | ||
const modifiedDate = new Date(dateString); | ||
return modifiedDate.toLocaleString(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These conversion functions may have to be slightly more complicated unfortunately. Using the Date.toLocaleString()
and .toUTCString()
functions as is would also change the formatting of the timestamps from something like 2023-07-21 01:14:44
to Fri, 21 Jul 2023 01:14:44 GMT
. Which might be an improvement, actually... But just something to consider.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a2227fd
to
2aae6ee
Compare
@kunalaswani this looks like it has stalled. Is it because you didn't ask for a re-review? (I have now done that.) |
export function convertToLocalTime(dateString) { | ||
const modifiedDate = new Date(dateString); | ||
return modifiedDate.toLocaleString(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -201,7 +220,7 @@ export const Files: React.FunctionComponent<FilesProps> = ({ | |||
MaxSkew: { | |||
label: nlsHPCC.MaxSkew, width: 60, formatter: React.useCallback((value, row) => value ? `${Utility.formatDecimal(value / 100)}%` : "", []) | |||
}, | |||
Modified: { label: nlsHPCC.ModifiedUTCGMT }, | |||
Modified: { label: nlsHPCC.ModifiedUTCGMT, formatter: currentTime }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missed this before, but you'd probably have to change the label here depending on which TZ is selected, so it doesn't always say "UTC"
@kunalaswani You'll need to rebase to the latest 9.2.x to take care of the conflicts GitHub is noting below. Hopefully there won't be too many conflicts that make it difficult. Also, something like this could address several of my previous comments about the timestamps not actually changing in the grid and the modified column's label not reflecting whether or not UTC was being used jeclrsg@a79880f?diff=unified&w=1 Or you might could use it as a reference at least. I should've mentioned, I did already rebase that branch of mine to latest 9.2. So the line numbers probably don't exactly match up to your branch now. |
Toggle Button added to switch between Local and UTC timezones. Signed-off-by: Kunal Aswani <[email protected]>
@kunalaswani GH Actions noted some lint issues, after rebasing. Also, there were still some pending comments from my last review. The main issue I was seeing while testing just now was that the state of the UTC isn't properly tracked, so the grid rows aren't updating. Might've been some artifact of rebasing; merge collisions or whatnot. But that branch I'd linked to in a previous comment showed one way you could fix it, by moving the UTC status into it's own state variable. And the "currentTime" callback also made the way the datetimes were printed be consistent regardless of the timezone being used in the grid. |
Toggle Button added to switch between Local and UTC timezones.
Type of change:
Checklist:
Smoketest:
Testing: