Experimental: Check for override attribute disable-preference-override
#331
Conversation
…r override attribute ignore-user-time-preference
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new ignore-user-time-preference attribute that allows specific <relative-time> elements to bypass the global data-prefers-absolute-time preference and maintain their relative formatting.
- Adds early return in
#shouldDisplayUserPreferredAbsoluteTimeto check forignore-user-time-preferenceattribute - Enables selective preservation of relative time formatting when global absolute time preference is enabled
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/relative-time-element.ts
Outdated
| 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 |
There was a problem hiding this comment.
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.
src/relative-time-element.ts
Outdated
| 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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
I think we could add it anyways -- it might make it easier for us to test dynamicbehavior in the browser :)
khiga8
left a comment
khiga8
left a comment
There was a problem hiding this comment.
Looking good! Can we add a test? Maybe somewhere in this suite!
src/relative-time-element.ts
Outdated
| 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 |
There was a problem hiding this comment.
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.
zaataylor
changed the title
ignore-user-time-preferencedisable-preference-override
Now, part of the check for determining if we should display user time preferences will also check for whether there is a specific attribute on the element,
disable-preference-override. If this attribute is present, we will use whatever the default behavior/display options are for the element, rather than displaying with absolute time.This will enable us to selectively override elements in certain UIs even when a user setting to enable absolute time has been enabled. This will allow us to roll out changes without breaking parts of the UI we already know can't handle longer timestamps well.