Skip to content

Vendor relative-time-element as local web component#36853

Draft
silverwind wants to merge 9 commits intogo-gitea:mainfrom
silverwind:reltime
Draft

Vendor relative-time-element as local web component#36853
silverwind wants to merge 9 commits intogo-gitea:mainfrom
silverwind:reltime

Conversation

@silverwind
Copy link
Member

@silverwind silverwind commented Mar 6, 2026

Replace the @github/relative-time-element npm dependency with a vendored, simplified implementation.

  • Support 24h format rendering PR 329
  • Enable ::selection styling in Firefox PR 341
  • Remove timezone from tooltips (It's always local timezone)
  • Clean up previous title workaround in tippy
  • Remove unused features

Replace the `@github/relative-time-element` npm dependency with a
vendored, simplified TypeScript implementation in `web_src/js/webcomponents/`.

Changes:
- Strip unused features: `format="micro"`, `format="elapsed"`,
  `data-prefers-absolute-time`, `RelativeTimeUpdatedEvent`, and
  `aria-hidden` span wrapping
- Apply hourCycle support from upstream PR go-gitea#329
- Apply `part="root"` for `::selection` styling from upstream PR go-gitea#341
- Remove timezone from date tooltips
- Disable `wc/no-constructor` eslint rule (needed for shadow DOM)

Co-Authored-By: Claude claude-opus-4-6 20250630 <noreply@anthropic.com>
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 6, 2026
silverwind and others added 2 commits March 6, 2026 18:43
Single-file vendored component, no separate module needed.

Co-Authored-By: Claude claude-opus-4-6 20250630 <noreply@anthropic.com>
Remove unused code paths identified by analyzing actual template usage:
- All property setters (never called from JS, config via HTML attributes)
- ListFormatPonyFill (Intl.ListFormat has universal browser support)
- milliseconds support (durationRe doesn't capture ms, precision always 'second')
- digital/2-digit DurationFormat styles (never used)
- Duration.from object branch (only string inputs used)
- noTitle, timeZoneName, timeZone, precision getters (never set)
- Corresponding observedAttributes entries

736 -> 559 lines (~24% reduction).

Co-Authored-By: Claude opus-4-6 v1 <noreply@anthropic.com>
silverwind and others added 2 commits March 6, 2026 18:57
Signed-off-by: silverwind <me@silverwind.io>
- Derive partsTable from unitNames (single source of truth)
- Remove redundant documentElement fallback (closest() traverses to html)
- Use static field for observedAttributes
- Create span once in constructor, update textContent directly
- Remove #updateRenderRootContent method

Co-Authored-By: Claude opus-4-6 v1 <noreply@anthropic.com>
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR replaces the @github/relative-time-element dependency with a locally vendored <relative-time> web component implementation, while simplifying features and adjusting tooltip/time formatting behavior in the web UI.

Changes:

  • Add a new vendored web_src/js/webcomponents/relative-time.ts implementation and register it via the webcomponents entrypoint.
  • Remove @github/relative-time-element from package.json / pnpm-lock.yaml.
  • Update selection styling to support ::part(root) and remove timezone abbreviations from formatted tooltip datetimes.

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
web_src/js/webcomponents/relative-time.ts New vendored <relative-time> implementation with shadow DOM + formatting logic.
web_src/js/webcomponents/index.ts Swap npm import for local web component import.
web_src/js/utils/time.ts Remove timeZoneName: 'short' from formatted datetime output.
web_src/css/base.css Add relative-time::part(root)::selection styling support.
package.json Drop @github/relative-time-element dependency.
pnpm-lock.yaml Remove lock entries for @github/relative-time-element.
eslint.config.ts Disable wc/no-constructor rule (currently globally).
Files not reviewed (1)
  • pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

- Fix epoch date handling: use explicit NaN check instead of falsy getTime()
- Fix locale fallback: use navigator.language instead of invalid 'default'
- Fix year comparison: use getFullYear() to match local-time formatter
- Fix customTitle: treat empty title as not custom (tippy sets title="")
- Scope wc/no-constructor disable to file instead of globally

Co-Authored-By: Claude opus-4-6 v1 <noreply@anthropic.com>
silverwind and others added 2 commits March 6, 2026 19:24
Title attribute changes now return early after updating customTitle flag,
preventing a cycle where update() sets title, tippy clears it, and the
attributeChangedCallback queues another update.

Co-Authored-By: Claude claude-opus-4-6 v1 <noreply@anthropic.com>
The relative-time component now sets data-tooltip-content and aria-label
directly, removing the intermediary title attribute. This eliminates the
interaction with tippy's MutationObserver that caused an infinite loop
(update sets title → tippy clears title → attributeChangedCallback fires
→ update runs again).

Also removes the relative-time special case and title watching from
tippy.ts since they are no longer needed.

Co-Authored-By: Claude claude-opus-4-6 v1 <noreply@anthropic.com>
@lunny
Copy link
Member

lunny commented Mar 6, 2026

Replace the @github/relative-time-element npm dependency with a vendored, simplified implementation.

  • Strip unused features: format="micro", format="elapsed", data-prefers-absolute-time, RelativeTimeUpdatedEvent, aria-hidden span wrapping, all property setters, ListFormatPonyFill, milliseconds support, digital/2-digit DurationFormat styles, unused attribute getters (noTitle, timeZoneName, timeZone, precision)
  • Apply hourCycle support from upstream PR #329
  • Apply part="root" for ::selection styling from upstream PR #341
  • Remove timezone abbreviation from date tooltips
  • Disable wc/no-constructor eslint rule (needed for shadow DOM attachment)

Generated by Claude

Please plain why we should vendor it. Whether a fork is better?

@silverwind
Copy link
Member Author

silverwind commented Mar 6, 2026

Because https://github.com/github/relative-time-element is not well-maintained anymore. The original author is no longer employed by GitHub and the module has been stagnating since then. And I want the 24h feature, but I doubt they will merge it.

@lunny
Copy link
Member

lunny commented Mar 6, 2026

Because github/relative-time-element is not well-maintained anymore. The original author is no longer employed by GitHub and the module has been stagnating since then. And I want the 24h feature, but I doubt they will merge it.

But how about create a fork rather then vendor it?

Signed-off-by: silverwind <me@silverwind.io>
@TheFox0x7
Copy link
Contributor

Since I'm not up to date on this area - are the cut features something that might be useful in the future?
Also I think tests would be good to have here.

@@ -0,0 +1,531 @@
// Vendored and simplified from @github/relative-time-element

Copy link
Member

Choose a reason for hiding this comment

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

If this is being vendored from upstream, could you include the license text in a comment in the header (assuming it is mit)?

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Mar 7, 2026
Copy link
Member

@techknowlogick techknowlogick left a comment

Choose a reason for hiding this comment

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

Meant to submit as comment, not send as PR approval. My apologies

@silverwind
Copy link
Member Author

silverwind commented Mar 8, 2026

Since I'm not up to date on this area - are the cut features something that might be useful in the future? Also I think tests would be good to have here.

Features can be re-added if needed. The whole module is only 500 lines (down from around 1100 lines of the original), I think it'll be managable. And I agree on tests, I will port a few.

@silverwind silverwind marked this pull request as draft March 8, 2026 06:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm/need 1 This PR needs approval from one additional maintainer to be merged. modifies/dependencies modifies/frontend

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants