Skip to content

Experimental: Check for override attribute disable-preference-override #331

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Experimental: Check for override attribute disable-preference-override #331

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d4e0921
when determining if we should display user time preferences, check fo…
zaataylor Nov 6, 2025
e454842
Rename attribute for user time preference override
zaataylor Nov 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/relative-time-element.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,9 @@ export class RelativeTimeElement extends HTMLElement implements Intl.DateTimeFor
// Never override duration format with absolute format.
if (format === 'duration') return false

// Override for user preferences; used sparingly to preserve intended relative formatting in some places
if (this.hasAttribute('ignore-user-time-preference')) return false
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

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

The new ignore-user-time-preference attribute is not documented in the README.md. The attributes table (lines 65-84) and attribute descriptions should include this new attribute, explaining its purpose to override the global data-prefers-absolute-time setting for specific elements. The inline comment on line 247 suggests this is intentional functionality that should be part of the public API documentation.

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

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

The ignore-user-time-preference attribute is not included in the observedAttributes array (lines 103-126), which means changes to this attribute after the element is created won't trigger re-rendering. If this attribute can be toggled dynamically, it should be added to observedAttributes to ensure the element updates when the attribute changes. If it's intentionally static, this should be documented.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

@zaataylor zaataylor Nov 6, 2025

Choose a reason for hiding this comment

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

I think it's okay that this is left out of the array. We won't be toggling this dynamically and would instead place this attribute intentionally on specific elements

Copy link
Contributor

@khiga8 khiga8 Nov 6, 2025

Choose a reason for hiding this comment

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

I think we could add it anyways -- it might make it easier for us to test dynamicbehavior in the browser :)

Copy link
Contributor

@khiga8 khiga8 Nov 6, 2025

Choose a reason for hiding this comment

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

Another thought after taking a closer look at existing code...

Can we try put this into a get, set similar to other attributes that are supported on relative-time?
Then calling this.newAttributeName here.


return (
this.ownerDocument.documentElement.getAttribute('data-prefers-absolute-time') === 'true' ||
this.ownerDocument.body?.getAttribute('data-prefers-absolute-time') === 'true'
Expand Down