Improve timestamp formatting #135
Conversation
|
Sorry for the delay chap, looks great! You can test on browser running on WSL or macos by installing Nix then running: nix run .#serve-gantz-websiteIt will then give you a localhost link you can test locally on your browser. It looks like as of #136 landing, there are some conflicts to resolve in the graph_select.rs module. Once you've tested the browser, if you could rebase this branch onto the latest master and resolve the conflicts that would be wicked 👌 |
- Change format from '2025-11-11T22:32:58Z' to '2025-11-29 14:32:58' - Add local timezone support using time crate - Implement WASM support via js-sys for browser timezone detection - Create shared to_local_datetime() helper function - Update LogView, TraceView, and GraphSelect widgets Closes nannou-org#132
mutualideas
force-pushed
the
improve-timestamp-formatting
branch
from
c82654f to
ee2acc5
Compare
|
Rebased onto latest master and resolved conflicts with #136. Testing
The implementation uses the |
| let system_time = crate::system_time_from_web(self.timestamp).expect("failed to convert"); | ||
| let datetime = OffsetDateTime::from(system_time); | ||
| let local_datetime = crate::widget::to_local_datetime(datetime); | ||
|
|
||
| let format = format_description::parse("[year]-[month]-[day] [hour]:[minute]:[second]") | ||
| .expect("invalid format"); | ||
|
|
||
| local_datetime | ||
| .format(&format) | ||
| .unwrap_or_else(|_| "<invalid-timestamp>".to_string()) |
There was a problem hiding this comment.
I'm noticing we have roughly the same pattern in trace_view.rs, log_view.rs and graph_select.rs - maybe we can abstract this into a standalone function that we can share between them? E.g. I'm thinking something like fn format_local_datetime(system_time) -> String { ... }, perhaps declared in the root of the crate or in the widget module alongside your to_local_datetime fn.
| Err(_) => datetime, | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Nice job on working this out! Tbh I'm surprised the time crate doesn't already have something like this for its UtcOffset::current_local_offset implementation.
Edit: Actually it looks like time does have something similar! Internally, UtcOffset::current_local_offset calls local_offset_at, which does have a wasm/js implementation here:
https://github.com/time-rs/time/blob/main/time/src/sys/local_offset_at/wasm_js.rs.
Its implementation looks really similar to what we have here. Can you double-check whether or not we can just use UtcOffset::current_local_offset for wasm32 too? It certainly looks like they're trying to support wasm at least.
There was a problem hiding this comment.
I actually tried using UtcOffset::current_local_offset() directly for WASM initially, but it was failing and falling back to UTC in the browser. The manual js-sys::Date.getTimezoneOffset() approach is what finally made it work correctly.
I'll dig into this a bit further - let me know if you have any ideas what might be causing this issue.
There was a problem hiding this comment.
Ahhhh interesting, thanks for the context. Yeah if that is the case, then it sounds like it could be a bug in the time crate potentially?
The only other thing that comes to mind that might be causing the incorrect offset to show is maybe if the browser used during testing had blocked location permissions or was set to the wrong timezone or something? Totally speculating though, I'm not even sure if any permissions are required to query the timezone in the browser.
2 participants
Description
Improves timestamp formatting in LogView, TraceView, and GraphSelect widgets to use a more readable format with local timezone offset.
Changes
2025-11-11T22:32:58Zto2025-11-29 14:32:58timecrate withwasm-bindgenfeature for cross-platform supportto_local_datetime()helper function to avoid code duplicationTesting
wasm32-unknown-unknowntarget without errorsNotes
The
timecrate'swasm-bindgenfeature should handle web platform compatibility automatically, but I wasn't able to verify this in a browser environment. The code compiles forwasm32-unknown-unknowntarget without errors.Closes #132