feat(Timestamp): show one instant across several time zones on hover (#4188)#4234
Draft
AKnassa wants to merge 3 commits into
Draft
feat(Timestamp): show one instant across several time zones on hover (#4188)#4234AKnassa wants to merge 3 commits into
AKnassa wants to merge 3 commits into
Conversation
Adds `tooltipEntries`, an additive array prop that renders the hover tooltip as one line per entry, each with an optional `timezoneID` (IANA id; omit it or pass 'local' for the viewer's zone), `format`, and `label`. Answers the API-design question in facebook#4188 through the repo's arbitration process: four candidate shapes were authored independently, vibe-tested against six naive prompts, and scored by three judge lenses. The entry-array shape won on conventions and on evidence. Behaviour: - Default is unchanged. With no entries the tooltip content stays the bare full-absolute string in the viewer's zone, with no wrapper element introduced around it. An empty array counts as no configuration, so `hasTooltip` remains the only on/off axis. - Configuring entries also attaches the tooltip to absolute formats, which previously had none. Leaving that gate closed would have let `format` silently suppress another prop's output. The cost is a tab stop and focus ring on those timestamps, as relative ones already have; opt-in, but a real tab-order change. - `format` accepts every non-relative `TimestampFormat` plus 'full'. 'full' lives only in the tooltip vocabulary, so `TimestampFormat` is untouched and future members become valid tooltip formats for free. - `system_*` lines honour an explicit zone via `formatToParts`, reached only when a zone is named; the existing local-getter path stays on every route an existing render can take, and a test pins the two to character-for-character equality. - An unknown zone id falls back to the viewer's zone with a warning rather than throwing a RangeError through render. - The accessible name is unchanged: it stays the canonical local absolute time, and the extra zones reach assistive tech through the tooltip's existing aria-describedby. Also corrects `isTimezoneShown`'s JSDoc, `.doc.mjs` (en/zh/dense) and its showcase block, which all claimed it applied to `system_date_time` and `system_time`. It never has. 32 new tests, all timezone-agnostic so they hold under any host zone. Part of facebook#4188
Edge-case pass over `tooltipEntries`. Two behaviour fixes, both found by testing rather than review: - An unknown zone id warned once per entry per render, so a single typo on an `isLive` timestamp emitted the same warning every tick forever. Each bad identifier is now reported once. - A lone entry naming a foreign zone rendered with no zone marker on `date_time`/`time`. Because the tooltip is the `<time>` element's aria-describedby and that element's accessible name is the absolute time in the *viewer's* zone, an unmarked foreign time was announced directly after a local one with nothing to tell them apart. A line whose zone the consumer named explicitly now carries its abbreviation; a line they did not steer stays bare, matching how the visible text reads with `isTimezoneShown` off. Two existing tests asserted the old marker rule. Their premise — that a marker on a single zone is noise — is what the second fix overturns, so they were retargeted at what the rule now guarantees rather than relaxed: one pins that the unnamed local line is also marked once a second zone appears (the case only the multiple-zones clause reaches), the other that two unnamed lines of the same zone stay bare. New coverage, all timezone- and locale-agnostic: midnight rendering as 00 rather than 24 under hourCycle h23; 45-minute offset zones; the +14/-11 extremes either side of the date line; year boundary; leap day; both halves of a repeated DST hour collapsing to one wall clock; empty and whitespace-padded zone ids; legacy link ids; distinct-zone counting when an entry falls back to local or differs only in casing; empty-string labels; and on the component, an unparseable value with entries, `auto` resolving to an absolute format, unix-seconds values, dt/dd pairing, the tab stop being surrendered when entries are removed, and the aria-describedby wiring that is the only route to assistive tech on absolute formats. Documents that fixed-offset ids like `'EST'` are accepted by the platform but never observe daylight saving, so they read an hour wrong for half the year. 93 Timestamp tests, up from 67. Suite 7472 pass / 0 fail. Part of facebook#4188
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
requested review from
cvkxx,
ernestt,
kentonquatman and
rubyycheung
July 23, 2026 03:22
Contributor
PR Analysis ReportModified ComponentsTimestamp
Bundle Size Summary
Accessibility AuditStatus: 1 accessibility violation(s) found — 1 serious. Timestamp - 1 issue(s)
Generated by PR Enrichment workflow | View full report |
An adversarial pass flagged that the distinctness key treats 'UTC' and 'GMT' as two zones when they are aliases for one, which on the first commit rendered two character-identical lines each carrying a zone abbreviation while a single entry carried none. The marker rule that landed since — a line whose zone the consumer named is marked whether or not a second zone is present — already removes the discrepancy, so there is no behaviour left to fix and no failing test to write. Canonicalizing the key would be a change no test could justify. Pinned instead: the alias pair and the single entry must render identically. Verified load-bearing — narrowing the rule back to "more than one zone" fails this test alongside the named-zone one. Part of facebook#4188
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this does
Hovering a timestamp can now show the same moment in several time zones and formats at once, each on its own labelled line. Today it always shows exactly one line, in the reader's own zone.
One new prop. Each entry is one line: an optional
timezoneID(omit it, or pass'local', for the reader's own zone), an optionalformat, an optionallabel.Why the shape is this shape
The issue says "needs API design", so this went through API Arbitration rather than my taste: four candidate APIs written independently, each given only its own docs to six naive authors who were asked for real product outcomes ("hovering should show the reader's zone and the incident's origin zone, clearly labelled"), then scored by three judges on different criteria.
Two calls I made against the panel, so they're easy to challenge:
renderTooltiprender prop. I didn't take it —renderTooltipis already taken inuseTooltip.tsx:198with a different contract, the issue's acceptance criteria ask for per-line labels as an API rather than delegated to each caller, and the bare-escape-hatch shape produced keyboard-unreachable tooltips in naive hands.Nothing changes unless you opt in
Without the new prop, the tooltip content is the same single string it is today — no wrapper element, no markup change. An empty array counts as no configuration, so
hasTooltipstays the only on/off switch.One deliberate behaviour change worth your eyes: configuring entries also turns the tooltip on for absolute formats, which have never had one. Leaving that shut would let
formatsilently suppress another prop's output, which the conventions wiki forbids. The cost is that those timestamps gain a tab stop and focus ring, exactly as relative ones already have — so a column of 500 of them gains 500 tab stops. Opt-in only, but it is a real tab-order change.'full'(the long absolute style) lives only in the tooltip vocabulary and never touchesTimestampFormat— so this has zero conflicting hunks with #4199, and thedate_long/date_weekdayyou're adding there become valid tooltip formats for free. I confirmed the tripwire by simulating that change: adding a member toTimestampFormatfails the build (TS2366) in both the new switch and the existingformatTimestamp, rather than silently returning nothing.Two things I need a call on
tooltipEntries,timezoneID,'local','full') is my reading of your open questions, not your decision. Happy to reshape.Found while testing, not fixed here
parseValuemisreads every pre-1970 millisecond timestamp: the heuristicvalue < 1e12 ? value * 1000 : valueis applied to a signed number, so a negative millisecond value gets multiplied again. The moon landing (-14182940000) renders as the year 1520. It's pre-existing, unrelated to tooltips, and changes behaviour for everyTimestampconsumer, so it wants its own PR — say the word and I'll open one.Fixed in passing, because it sits on the prop next door:
isTimezoneShown's JSDoc,.doc.mjs(en/zh/dense) and its showcase block all claimed it applied tosystem_date_timeandsystem_time. It never has —formatTimestamphas no such branch, so the showcase was demonstrating a no-op.How to check it
Storybook → Core/Timestamp → Tooltip — multiple time zones: local+UTC, three labelled zones, one zone in two formats, and a UTC-only audit log. Hover or tab to each.
Worth knowing: the tooltip is capped at 300px, so the long
fullstyle plus a label wraps. The docs steer multi-zone tooltips todate_time, which fits on one line.Testing
93 Timestamp tests, up from 32 before this branch. All timezone- and locale-agnostic, because the repo pins
TZin neither vitest nor CI — a dev machine and a UTC runner have to agree, so exact assertions only use explicit-zonesystem_*output.Covered: the date line (
+14/-11, 25 hours apart), 45-minute offsets, midnight rendering as00and not24, both halves of a repeated DST hour, year and leap-day boundaries, empty and whitespace-padded zone ids, legacy link ids, casing, unknown ids degrading to the reader's zone instead of throwing, an unparseable value,autoresolving to an absolute format, and thearia-describedbywiring that is the only route to assistive tech on absolute formats.Eleven mutations were run against the implementation; every one was caught by a test. Full suite 7472 pass / 0 fail (baseline on
d762f825dwas 7414 / 0 — no pre-existing failures to excuse). Core and Storybook typecheck clean, ESLint clean, changeset valid.Closes #4188