feat: initial implementation#1
Conversation
|
Requesting review @javier-godoy @scardanzan |
javier-godoy
left a comment
There was a problem hiding this comment.
Code review — automated (recall-oriented)
Reviewed the full diff and verified the entire attribute vocabulary against the upstream @github/relative-time-element v5.0.0 source (format, tense, precision, format-style, time-zone/time-zone-name, per-part attributes, prefix, no-title, isDuration, shadow-root rendering). All enum→attribute mappings match upstream and all written attributes are in upstream's observedAttributes.
Verdict: a clean, well-tested thin wrapper — no functional bug in the core mapping logic. The items below are one real validation gap, a misleading HasSize, two doc-vs-code contradictions, and some edge/cleanup nits. Inline comments mark each location.
Findings (ranked)
- Per-part setters don't validate the part/style subset → invalid values silently dropped/coerced in the browser. (correctness —
RelativeTime.setWeekdayet al.) setTimeZone(ZoneId)with aZoneOffset/legacy zone emits+02:00/GMT+02:00→ clientRangeError; no IANA guard. (correctness, edge)HasSizeis a no-op on the inline host; SPEC §5 wrongly advertises it for layout. (API/doc)- Javadoc + SPEC §2.2 claim offset/zone is "preserved on the wire string" — false; normalized to UTC via
toInstant(). (doc contradiction) - SPEC §2.4
// → threshold="P30D"is wrong;Duration.ofDays(30).toString()=PT720H. (doc error) setDateTime(null)is ambiguous across 5 overloads → won't compile without a cast; noclear(). (ergonomics)setLocale(Locale.ROOT)→lang="und", silent fallback, undocumented. (edge)- Negative
Durationthreshold →PT-nSrejected by upstreamisDuration→ silent revert to defaultP30D. (edge) - Identical
attributeValue()duplicated across 4 enums. (cleanup) setNoTitleis the one setter that bypasses thesetOrRemovehelper. (cleanup)
Spec text to change
- §2.7 ("invalid combinations… silently falls back") describes a gap to close (#1), not a contract.
- §5
HasSizeclaim is inaccurate (#3). - §2.2 offset/zone rows and §2.4
P30Dexample are wrong (#4, #5).
Checked & not issues
@CssImport("./styles/…") path (correct for META-INF/frontend); IT navigation/matcher; stopwatch button text; session-expiry double Instant.now() (intentional); duplicate AppShellConfigurator (only one — standard FC idiom); pom.xml scanIntervalSeconds→scan (both 0 = disabled; correct Jetty 12 param).
Recommendation: approve with changes — fix #1 and decide #3; correct the docs for #4/#5.
|
@javier-godoy — ready for human review. 🙋 Re-reviewed the pushed fixes (incremental diff
Remaining notes are all low-severity polish (none blocking): a Javadoc caveat + caching for the timezone check, wrapping My automated assessment: approve pending your call on the open item and the polish notes. Over to you for human review. |
Relative Time Add-On: initial implementation
Implements the
RelativeTimecomponent: a thin Vaadin Flow wrapper around the @github/relative-time-element web component (v5.0.0). It renders a date/time as a human-readable relative string ("4 hours ago", "in 2 weeks") that updates itself in the browser as time passes.Component API
RelativeTime extends Component, configured through a fluent, typed Java API:setDateTimeoverloads + 6 constructors acceptingInstant,OffsetDateTime,ZonedDateTime,LocalDateTime,LocalDate(the local types interpreted in the server's default zone).getDateTime()returns the last value as a UTCInstant.setTense,setFormat,setPrecision,setFormatStyle,setThreshold(Duration),setPrefix,setNoTitle,setLocale.format=DATETIME):setTimeZone(ZoneId),setTimeZoneName, and per-part formatting setterssetYear/setMonth/setDay/setWeekday/setHour/setMinute/setSecond.nullto clear; all returnthisfor chaining.Tense,Format,Precision,FormatStyle,TimeZoneName,DateTimePartStyle.The wrapper drives the element via HTML attributes (not DOM properties), matching the upstream's attribute-driven design. This is documented in the class Javadoc and spec so it isn't accidentally "modernised."
Compatibility & theming
v25profiles).Tests
RelativeTimeTest: 19 unit tests covering every setter's attribute output, conversions, null-clearing, and fluent return.SerializationTest: empty + fully-configured instances.ViewIT: integration test; matcher updated to detect upgrade via shadow-root text content (relative-time renders text, not child elements).Demos
Two tabs sharing an
AbstractRelativeTimeDemobase (separator + section-highlight helpers):Adds SPECIFICATIONS.md documenting the full API, defaults, live-update behaviour, and known upstream quirks.
🤖 Generated with Claude Code