Synced from GitHub
fix(web): respect 24-hour time entry in custom snooze#22433
Custom snooze ignored Settings → Time format → 24-hour and could still display an AM/PM field. Explicit 24-hour mode now uses an HH:MM text field with 00:00–23:59 validation, so the visible format follows the setting instead of the native picker's locale behavior. Other time-format settings keep the existing native picker. Snooze date resolution, future-time validation, and duration behavior are unchanged.
Addresses the 24-hour case in #19930. This is a small correction to an existing explicit setting, under the focused obvious-bug exception in CONTRIBUTING.md.
I think #12440 is the better solution because it provides a custom UI. I do not know why it was closed. This PR is a narrower fix for respecting the explicit 24-hour setting.
Verified on Linux with Chromium 148.0.7778.96 in the repository's browser-test harness:
cd apps/web && vp test run --project browser src/components/CustomSnoozeDialog.browser.test.tsx: 5 passed. Covers visible 17:30 entry, 23:59, midnight on a future date, invalid/empty/past times, and native entry for 12-hour and system-default settings.vp test run packages/client-runtime/src/state/customSnooze.test.ts apps/web/src/components/Sidebar.snooze.test.ts: 28 passed, including local-time/DST validation and duration semantics.- Targeted
vp lintandvp fmt --checkfor both changed files passed. vp run --filter @t3tools/web typecheckpassed, with existing Effect suggestions elsewhere.
Both screenshots show the actual dialog with the 24-hour setting and the same 17:30 input. Before uses the main-branch component; after uses this fix. The native field's underlying value was already 17:30, but it visibly displayed 05:30 PM. The text field visibly displays 17:30.
| Before | After |
|---|---|
| Before: native time field displays 05:30 PM despite the explicit 24-hour setting | After: time field visibly displays 17:30 |
Integrated app/Electron verification was not completed. T3's Browser panel could not launch because the running T3 host is root with Chrome's sandbox enabled. The screenshots and interaction tests above use the existing Chromium component-test harness; Electron itself was not launched. Web and desktop share this dialog. Mobile uses separate native pickers and is outside this web/desktop fix.
Note
This PR was implemented by gpt-6.1-sol (high reasoning effort) in Codex.
Takeover verification at 00c31cc6a61756a1cc81b55d6256f78bd66a2377:
Merged current main without rebasing, preserving its clock updates and snooze-settings link. Updated the existing browser test to use the full fake clock required by current lint. The change remains two files, 97 additions and one deletion relative to main.
Blacksmith vp run check:changed github/main passed format, lint, knip, affected typechecks, all five browser tests, and React Compiler coverage. In the real web client, verified the 24-hour setting, rejected 24:00, submitted 23:59, persisted snooze across reload, and woke the thread again. Also checked native 12-hour entry, duration mode, cancel, the settings link, and the narrow dark layout. Electron was not launched; mobile pickers are unchanged. Two independent reviews approved this head.
24-hour entry rejects an invalid time and snoozes the thread with 23:59
Takeover by gpt-6.1-sol through the Codex harness.
Checks
22 successful · 7 skipped
Synced from GitHub
Approvability
Verdict: Approved at 00d46c6
Macroscope's review found this PR approvable — This is a small, self-contained UI bug fix that makes the existing 24-hour setting use an HH:MM field while preserving the native picker for other formats. Snooze resolution and defaults are unchanged, and focused browser tests cover the new behavior.
You can add or adjust custom eligibility rules. Learn more.
Added 1 commit
- fix(web): respect 24-hour time entry in custom snoozeb409d2c
Synced from GitHub
No actionable comments were generated in the recent review.
External image Recent review info
-
External image Run configuration
- Source mode: Auto
- Repository: pingdotgg/t3code
- Branch: main
- File: .coderabbit.config.ts
- Commit: 7195b69
- Layers: Repository TS config
- Review profile: CHILL
- Plan: Advanced
- Run ID:
b3cf46aa-7f76-484e-a5a8-60e71c4fa62a
External image Commits
Reviewing files that changed from the base of the PR and between b409d2cf02943e4123c922857fad54b75f228ad1 and 00c31cc6a61756a1cc81b55d6256f78bd66a2377.
External image Files selected for processing: 2
apps/web/src/components/CustomSnoozeDialog.browser.test.tsxapps/web/src/components/CustomSnoozeDialog.tsx
Limit details: You’ve used all 10 included reviews currently available.
Walkthrough
The custom snooze dialog reads the configured timestamp format. For 24-hour format, it uses a text field with
HH:MMvalidation. For other formats, it retains the native time input. Browser tests cover input behavior and submitted timestamps.Changes: Custom snooze time format
Layer / File(s) Summary Format-aware input and validation
apps/web/src/components/CustomSnoozeDialog.tsx,apps/web/src/components/CustomSnoozeDialog.browser.test.tsxThe dialog uses a text input with HH:MMvalidation when 24-hour format is configured. Other formats retain the native time input. Browser tests cover valid and invalid times and verify submitted timestamps.Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix · Severity of issue fixed: Low
Possibly related issues
#27233— This issue also reports a time-format mismatch, but its ScheduledTasksSettings path is not changed here.
Suggested labels:
bugfixSuggested reviewers:
juliusmarminge,t3dotgg
Merge Risk: External image Minimal · up to 00c31
24-hour mode gains validated text entry; 12-hour behavior remains unchanged. No merge-blocking regression was identified.
Pre-merge checks: Passed 3 · Failed 1
Failed Failed checks: 1 warning
Check name Status Explanation Resolution Linked Issues check Warning [ #19930] Requirement: the custom snooze time picker must honor Settings → Time format. Expected: 24-hour shows 17:30, 12-hour shows 5:30 PM, system default follows the locale.
Met (24-hour): At the r…Make the explicit 12-hour setting independent of the browser locale. For example, render a text field with hh:mm and AM/PM validation. Convert the entered value to local 24-hour time before the existing future-time check. Keep the native pi… Passed Passed checks: 3 passed
Check name Status Explanation Title check Passed The title clearly and concisely describes the main change: making custom snooze respect 24-hour time entry settings. Out of Scope Changes check Passed The whole-PR diff against the merge base 7195b69 changes two files. apps/web/src/components/CustomSnoozeDialog.tsx (+7/-1) adds the timestampFormat selector and the 24-hour text-field attributes. The … Description check Passed The description is complete and relevant. It explains the problem, change, scope exception, linked issue, verification results, screenshots, limitations, and agent usage. Although it does not use the … Full details: Out of Scope Changes check
Explanation
The whole-PR diff against the merge base 7195b69 changes two files. apps/web/src/components/CustomSnoozeDialog.tsx (+7/-1) adds the timestampFormat selector and the 24-hour text-field attributes. The new apps/web/src/components/CustomSnoozeDialog.browser.test.tsx (+90) covers the 24-hour, 12-hour, and locale behavior. Both changes connect directly to issue
#19930. The reviewed evidence shows no unrelated product changes. The full changed-file list was truncated in the files listing. The two-file scope relies on the supplied summary and the two file diffs, which matched that summary.
Full details: Description check
Explanation
The description is complete and relevant. It explains the problem, change, scope exception, linked issue, verification results, screenshots, limitations, and agent usage. Although it does not use the template's section headings, it includes the required information.
Full details: Linked Issues check
Explanation
[
#19930] Requirement: the custom snooze time picker must honor Settings → Time format. Expected: 24-hour shows 17:30, 12-hour shows 5:30 PM, system default follows the locale.Met (24-hour): At the reviewed head, CustomSnoozeDialog.tsx reads timestampFormat through useClientSettings. For 24-hour, the time field uses type=text, placeholder HH:MM, pattern ([01][0-9]|2[0-3]):[0-5][0-9], and a title. The browser test covers visible 17:30 and 23:59 entry, invalid, empty, and past values, and midnight on a future date.
Met (system default): The field keeps type=time, so the browser locale controls the display.
Not met (12-hour): For 12-hour, the field keeps type=time. The PR description states that the native picker follows locale behavior. On a browser or OS with a 24-hour locale, a 12-hour setting therefore displays 17:30, not 5:30 PM. The 12-hour browser test checks only the type attribute and the submitted value. It does not check the displayed format. The author narrowed the scope to 24-hour. The issue requirement still names 12-hour mode, and the PR does not satisfy it on 24-hour locales.
Resolution
Make the explicit 12-hour setting independent of the browser locale. For example, render a text field with hh:mm and AM/PM validation. Convert the entered value to local 24-hour time before the existing future-time check. Keep the native picker for system default. Add a browser test that asserts the displayed 12-hour value, such as 5:30 PM, for an entry of 17:30 under the 12-hour setting.
Finishing Touches
Generate unit tests (beta)
- Create a new PR
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Added 2 commits
Note
Written by gpt-6.1-sol on behalf of Maria
Verified in a real dev environment
Verified unchanged head 00c31cc6a61756a1cc81b55d6256f78bd66a2377 on a disposable Linux VM using repository Playwright and /usr/bin/google-chrome against vp run dev.
Reproduced on github/main (ce9e016406) in a separate worktree: Settings → General → Time format → 24-hour, then thread actions → Snooze → Custom displayed 05:30 PM for the underlying 17:30 value. The exact PR head visibly displays 17:30 with the same date and app-created thread.
Created a project pointing at the real repository through the app. Real Codex gpt-6.1-sol turns completed through the web composer on both main and the PR. No mocks, replay provider, fixtures, or demo data were used. Only one dev server ran at a time.
The PR rejected 24:00 and today's past 00:00, accepted 17:30, persisted snooze across reload, and woke the thread through Wake now. Two-hour duration snooze, native 12-hour entry, cancel, and Edit snooze options navigation also worked.
text
| 1 | main Codex response: snooze verification ready. |
| 2 | PR Codex response: pr snooze verification ready. |
| 3 | PASS invalid 24:00 rejected, dialog remains open |
| 4 | PASS past time rejected |
| 5 | PASS 17:30 snoozes real thread |
| 6 | PASS snooze persists across reload |
| 7 | PASS wake, duration, native 12-hour entry, cancel, settings navigation |
| 8 | blacksmith: vp run check:changed github/main |
| 9 | PASS format, lint, knip, affected typechecks, five browser cases, React Compiler |
No source changes were needed. Existing CI reports 29 successful checks.
Before on main: 24-hour setting displays 05:30 PM
After on PR: the same date and time visibly display 17:30
Real app rejects invalid and past times, then snoozes with 17:30
Not verified
Electron and native iOS/Android clients were not launched. macOS/Windows locale behavior and automatic wake at the scheduled deadline were not exercised. Mobile has separate native pickers. The existing explicit 12-hour setting still relies on the browser locale; this focused PR changes only explicit 24-hour entry.