-
Notifications
You must be signed in to change notification settings - Fork 187
Experimental feature: Support absolute time format #327
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Co-authored-by: Zachary Taylor <zaataylor@github.com>
| return `${this.prefix} ${formatter.format(date)}`.trim() | ||
| } | ||
|
|
||
| #getUserPreferredAbsoluteTimeFormat(date: Date): string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now, matching the title format.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For other potential reviewers, here's an example of what this looks like: Oct 23, 2025 6:59 AM EDT
|
|
||
| #shouldDisplayUserPreferredAbsoluteTime(format: ResolvedFormat): boolean { | ||
| // Never override duration format with absolute format. | ||
| if (format === 'duration') return false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think... this makes sense? @zaataylor check me on this :D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes sense! 🫡 This will also work for elapsed format since they're aliases
relative-time-element/src/relative-time-element.ts
Lines 150 to 151 in a917f04
| // elapsed is an alias for 'duration' | |
| if (format === 'elapsed') return 'duration' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh and for micro, too:
relative-time-element/src/relative-time-element.ts
Lines 152 to 153 in a917f04
| // 'micro' is an alias for 'duration' | |
| if (format === 'micro') return 'duration' |
| this.ownerDocument.documentElement.getAttribute('data-prefers-absolute-time') === 'true' || | ||
| this.ownerDocument.body?.getAttribute('data-prefers-absolute-time') === 'true' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
adding support for body and html, based on @dgreif comment
There was a problem hiding this 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 introduces an experimental feature to support absolute time formatting based on user preference. When the data-prefers-absolute-time attribute is set to "true" on either the html or body element, relative time elements will display in absolute format instead of relative format.
Key Changes:
- Added logic to detect and respect the
data-prefers-absolute-timepreference from document or body elements - Implemented absolute time formatting with locale-aware date/time display
- Added comprehensive test coverage for the new feature including edge cases
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/relative-time-element.ts | Implements the core absolute time formatting logic and preference detection |
| test/relative-time.js | Adds test suite covering various scenarios for the absolute time feature |
| package.json | Reorders lint commands to run TypeScript checks before ESLint |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, these changes look good to me!
| return `${this.prefix} ${formatter.format(date)}`.trim() | ||
| } | ||
|
|
||
| #getUserPreferredAbsoluteTimeFormat(date: Date): string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For other potential reviewers, here's an example of what this looks like: Oct 23, 2025 6:59 AM EDT
|
|
||
| #shouldDisplayUserPreferredAbsoluteTime(format: ResolvedFormat): boolean { | ||
| // Never override duration format with absolute format. | ||
| if (format === 'duration') return false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes sense! 🫡 This will also work for elapsed format since they're aliases
relative-time-element/src/relative-time-element.ts
Lines 150 to 151 in a917f04
| // elapsed is an alias for 'duration' | |
| if (format === 'elapsed') return 'duration' |
|
|
||
| #shouldDisplayUserPreferredAbsoluteTime(format: ResolvedFormat): boolean { | ||
| // Never override duration format with absolute format. | ||
| if (format === 'duration') return false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh and for micro, too:
relative-time-element/src/relative-time-element.ts
Lines 152 to 153 in a917f04
| // 'micro' is an alias for 'duration' | |
| if (format === 'micro') return 'duration' |
Relates to: https://github.com/github/fixing-oss-pain-points/issues/148
Warning
This is an experimental feature. It may be modified or removed at any time!
This PR adds support for rendering absolute time format if the consumer has a
data-prefers-absolute-timeattribute on thebodyorhtmltag, set totrue.