Allow TimestampFormatter to show as a fully cacheable time difference with JS

The recent changes all look good to me. I had a little concern with the last commit, but after a back-and-forth with myself, I'm happy!

Swapping the Drupal Global Contribution Weekend 2023 tag with the correct one.

Left some minor feedback on the MR

@lauriii, fixed your suggestion and answered the other 2 remarks. Setting back to RTBC.

Looks like @lauriii's last comment on the MR still needs to be resolved.

I've only removed the class and, as this features already crossed RTBC several times I dare to se as RTBC.

Given that the rendered time is updated occasionally via JavaScript do we need to add aria-live attribute to it?

This is a complex new feature is constantly moved back and forth with improvement requests that could be solved in followups. Could we, please, postpone this potential improvement to a followup? It becomes more and more expensive to keep this MR alive and aligned

Reviewed the Patch on MR !570, worked for me.

Considering #235 , checked the rendered time is updated occasionally via JavaScript, yes it needs accessibility work, will try to work on it.

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

I don't believe the bot, so I scheduled a retest

"Can not add test, MR is not mergeable"

Bot was right. A couple of merge conflicts in the rebase to today's 10.1.x branch. All of them in core/lib/Drupal/Core/Field/Plugin/Field/FieldFormatter/TimestampFormatter.php in the settingsForm() method. Attached an example of one of the conflicts. I think I resolved it all properly, but I haven't tested. Let's see what the bot says now.

It was just a reroll

Given #235 and #236, I checked whether aria-live was an accessibility requirement here or not.

Docs at https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/ARIA_Liv...

Normally, only aria-live="polite" is used. Any region which receives updates that are important for the user to receive, but not so rapid as to be annoying, should receive this attribute. The screen reader will speak changes whenever the user is idle.

aria-live="assertive" should only be used for time-sensitive/critical notifications that absolutely require the user's immediate attention. Generally, a change to an assertive live region will interrupt any announcement a screen reader is currently making. As such, it can be extremely annoying and disruptive and should only be used sparingly.

As aria-live="off" is the assumed default for elements, it should not be necessary to set this explicitly, unless you're trying to suppress the announcement of elements which have an implicit live region role (such as role="alert").

Given this can potentially update very frequently, and it doesn't add new information to the page, it just silently updates information that's already there so that it's up-to-date if you leave a tab open for a long time or similar, I don't think we should add aria-live at all, if we did, we'd be setting it to aria-live="off" which is discouraged.

I am not really qualified to review the JavaScript in this issue, although to the extent that I can, I really like the approach of optimising the refresh to the maximum possible interval - that should hopefully make this cheap on people's tabs.

I did find one issue with the layout update path that needs fixing before this can go in - see comments on the MR. Marking needs work for that.

The new layout_builder_entity_view_display_presave function as well as the updates to layout_builder_post_update_timestamp_formatter look like what was requested by @catch.

I also like to change from strpos to str_starts_with, which was a nice touch.

Back to RTBC, hopefully for the last time. ;)

Re #236 I think accessibility is not an improvement but something that needs to be done by default every time a new feature is delivered. I actually came to the same conclusion as #243. Just wanted someone with proper experience confirmed this.

Thanks that looks better.

As mentioned I'm not qualified to sign off on the JS, but I checked who'd reviewed, and it's had extensive review from nod_ and droplet and others, so I think that's covered, and what I can review looked very good.

The PHP side looks great, it adds some extra form options, but if we didn't have them, I'd probably have asked for them, so I think it's good to have the flexibility those provide.

Did my best with the issue credits here but it's a very long issue with a lot of contributors, so apologies if I missed anyone.

Tagging for 10.1.0 release highlights.

Fixing tag

Automatically closed - issue fixed for 2 weeks with no activity.

I've opened 🐛 TimestampFormatter / time_diff missing config schema Needs work that is encountered w/ use of these updates.