Skip to content

fix: format chrono form field values the way browsers expect - #709

Open
m4tx wants to merge 2 commits into
masterfrom
fix-chrono-fields
Open

m4tx wants to merge 2 commits into
masterfrom
fix-chrono-fields

Conversation

@m4tx

@m4tx m4tx commented Oct 16, 2026

Copy link
Copy Markdown
Member

Description

Datetime and time values were rendered with Display, producing values like 2025-05-27 12:34:56.789 +00:00 that <input type="datetime-local"> rejects. Browsers then showed an empty input, so re-submitting a form (e.g. editing an object in the admin panel) silently cleared the field.

Type of change

  • Bug fix
  • New feature
  • Documentation
  • Refactor / cleanup
  • Performance improvement
  • Other (describe above)

@m4tx
m4tx requested review from ElijahAhianyo and seqre and a balanced review from Copilot October 16, 2026 07:06
@github-actions github-actions Bot added the C-lib Crate: cot (main library crate) label Oct 16, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Serialization can shift values with configured timezones and silently discard milliseconds.

2 open findings
What changed in this PR

Updates chrono form serialization to prevent browsers from displaying empty datetime inputs when editing existing values.

Changes:

  • Uses browser-oriented datetime and time formats.
  • Serializes timezone-aware datetimes in UTC.
  • Adds serialization and round-trip tests.
File Description
cot/​src/​form/​fields/​chrono.rs Updates chrono value formatting and adds regression tests.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread cot/src/form/fields/chrono.rs Outdated
Comment thread cot/src/form/fields/chrono.rs Outdated
@github-actions

github-actions Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

🐰 Bencher Report

Projectcot
Branchfix-chrono-fields
Testbedgithub-ubuntu-latest
Click to view all benchmark results
BenchmarkLatencyBenchmark Result
microseconds (µs)
(Result Δ%)
Upper Boundary
microseconds (µs)
(Limit %)
empty_router/empty_router📈 view plot
🚷 view threshold
7,997.20 µs
(-34.96%)Baseline: 12,296.05 µs
18,508.91 µs
(43.21%)
json_api/json_api📈 view plot
🚷 view threshold
564.16 µs
(-43.96%)Baseline: 1,006.77 µs
1,345.38 µs
(41.93%)
nested_routers/nested_routers📈 view plot
🚷 view threshold
572.23 µs
(-39.83%)Baseline: 951.05 µs
1,254.98 µs
(45.60%)
single_root_route/single_root_route📈 view plot
🚷 view threshold
523.13 µs
(-42.52%)Baseline: 910.18 µs
1,209.60 µs
(43.25%)
single_root_route_burst/single_root_route_burst📈 view plot
🚷 view threshold
12,100.00 µs
(-28.02%)Baseline: 16,811.28 µs
22,263.31 µs
(54.35%)
🐰 View full continuous benchmarking report in Bencher

@codecov

codecov Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.45652% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
cot/src/form/fields/chrono.rs 99.45% 1 Missing ⚠️
Flag Coverage Δ
rust 90.76% <99.45%> (+0.11%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
cot/src/form/fields/chrono.rs 95.49% <99.45%> (+2.16%) ⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@m4tx
m4tx force-pushed the fix-chrono-fields branch from a5060c3 to 35148e4 Compare October 16, 2026 07:44
@m4tx
m4tx requested a balanced review from Copilot October 16, 2026 10:46

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Internal serialization loses precision, and rendered timezone-aware values can conflict with HTML bounds.

2 open findings
2 resolved since last review

🧠 Review effort: Balanced

Comment thread cot/src/form/fields/chrono.rs Outdated
Comment on lines +412 to +413
let tz = self.custom_options.timezone.unwrap_or(Tz::UTC);
format_browser_datetime(&date_time.with_timezone(&tz).naive_local())
Comment thread cot/src/form/fields/chrono.rs Outdated
/// when rendering the `datetime-local` input.
fn to_field_value(&self) -> String {
self.to_string()
self.to_rfc3339_opts(SecondsFormat::Millis, false)
@m4tx
m4tx requested a balanced review from Copilot October 19, 2026 19:08

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

m4tx added 2 commits October 20, 2026 00:54
Datetime and time values were rendered with `Display`, producing values
like `2025-05-27 12:34:56.789 +00:00` that `<input type="datetime-local">`
rejects. Browsers then showed an empty input, so re-submitting a form (e.g.
editing an object in the admin panel) silently cleared the field.
@m4tx
m4tx force-pushed the fix-chrono-fields branch from a115cf9 to 460f013 Compare October 19, 2026 22:54
@m4tx
m4tx requested a balanced review from Copilot October 19, 2026 22:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@m4tx
m4tx requested a balanced review from Copilot October 20, 2026 09:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-lib Crate: cot (main library crate)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants