Adds noTitle option to RelativeTime component

[lindseywild]

Blocked until a new release of https://github.com/github/relative-time-element ships and is updated in primer/react.

Closes https://github.com/github/accessibility-audits/issues/5633

Changelog

New

Changed

Removed

Rollout strategy

• Patch release
• Minor release
• Major release; if selected, include a written rollout or migration plan
• None; if selected, include a brief description as to why

Testing & Reviewing

Merge checklist

• Added/updated tests
• Added/updated documentation
• Added/updated previews (Storybook)
• Changes are SSR compatible
• Tested in Chrome
• Tested in Firefox
• Tested in Safari
• Tested in Edge
• (GitHub staff only) Integration tests pass at github/github (Learn more about how to run integration tests)

[changeset-bot]

🦋 Changeset detected

Latest commit: 33f9c8d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@primer/react	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[github-actions]

size-limit report 📦

Path	Size
packages/react/dist/browser.esm.js	89.45 KB (+0.02% 🔺)
packages/react/dist/browser.umd.js	89.74 KB (-0.02% 🔽)

[lindseywild]

Integration failures are unrelated to this PR!

[siddharthkp]

Hi! 👋

For the docs, can you also add when is it recommended to skip title?

[lindseywild]

In this description? Or are you talking about updating the in-progress docs PR?

[siddharthkp]

Ideally both places, but either is fine for now :)

[siddharthkp]

Hi!

Can i request additional context, what does passing no-title down do? When is it recommended to use + when is it not recommended to use?

Based on https://github.com/github/accessibility-audits/issues/5633, trying to understand if this should be the default behavior or only in specific cases? (which ones?)

[lindseywild]

@siddharthkp Hi! Good question. Passing no-title tells the relative-time-element not to render a title attribute. title attributes are inaccessible by default, but we can't fully remove it from relative-time-element without a bigger conversation since it's all over GitHub and people use it often. If it helps, I have a PR for updated guidance in the works that I will merge after this one is merged :)

Ideally it'd be default behavior, but we'd need a larger conversation with stakeholders and design to come up with an accessible solution (we've attempted this in Office Hours, but there is nothing straightforward that we can do other than this without extensive testing).

This is also to match the behavior introduced in PVC.

[joshblack]

Would it be preferable to always have the title not be present, at least from the Primer React side? Internally we could set noTitle on the relative time element but we wouldn't expose it as a prop to consumers to toggle.

Just wanted to ask to see if this would be helpful or if not because of what you shared about usage at GitHub and how there isn't a good alternative yet when the title is not displayed 👀

[lindseywild]

Would it be preferable to always have the title not be present, at least from the Primer React side? Internally we could set noTitle on the relative time element but we wouldn't expose it as a prop to consumers to toggle.

Yes, it would be preferable to never have a title attribute! I added a story that shows how we can instead use a Tooltip on interactive elements, but that doesn't solve the case of non-interactive elements :/ If you think it's ok I am totally fine removing the title attribute altogether, but I think that may ruffle some feathers and we'd would probably require user testing first 😅

[siddharthkp]

If it helps, I have a PR for updated guidance in the works that I will merge after this one is merged :)

Yes, this was very helpful!

Please correct me if I understood this incorrectly:

• For visual users, the date is presented as "3 days ago" and then they can hover for exact date + time
• For screen reader users, the date is also presented as "3 days ago", but there is no way to know exact date + time because title is ignored.

(this is where I'm probably understanding incorrectly)

To make both experiences similar, we need to replace title with an accessible element, like a tooltip. By removing title without replacing it with tooltip, we do make the experience the same, but don't we lose the detailed information about data and time?

---

Ideally it'd be default behavior, but we'd need a larger conversation with stakeholders and design to come up with an accessible solution (we've attempted this in Office Hours, but there is nothing straightforward that we can do other than this without extensive testing).

I see where you are coming from! I feel a little worried about noTitle because I don't know how it or where will be used in the product. Can you share a little more on that.

How do you feel about primer components for RelativeTime (both PRC and PVC) using a tooltip internally instead of expecting it to be added in the product? It would lead to some inconsistency in the short term (between primer vs non-primer relative time), but hopefully will set the direction towards accessible + consistent

[lindseywild]

@siddharthkp You are mostly correct with how you're understanding it! The other user we're excluding is keyboard users -- even if a user can navigate to the element because it's focusable, like a link, the title attribute does not display for them.

but don't we lose the detailed information about data and time?

No, if we pass in the full date and time as the tooltip (check out the story Link with Tooltip) that shows the full date in the tooltip, but the shortened version as the main text.

I feel a little worried about noTitle because I don't know how it or where will be used in the product. Can you share a little more on that.

Sure! We can think of this as a "baby step" to a fully accessible solution. Since we don't have one right now, this is better than what currently exists, but still not fully ideal. One example of where this can be used in the product is on PRs, where the timestamp acts as a link to that comment. We can use noTitle to not render a title attribute, and instead replace it with a Tooltip with the full time as it is in the example story.

I am also thinking that if audit issues start coming in, similar to the one linked in the PR, it gives teams a way to fix the audit issue with minimal impact or needing to reassign the issue to Primer -- they have a few options of how to address it. They can do one of the following:

• Pass in noTitle so they don't render a title (and we can let the teams decide if they want to replace it or go without it)
• For interactive elements, pass in noTitle and pass in the full date/time in a Tooltip
• For non-interactive elements, pass in noTitle and write as much of the full date/time out as possible

How do you feel about primer components for RelativeTime (both PRC and PVC) using a tooltip internally instead of expecting it to be added in the product?

We considered this! The tricky part is that not all RelativeTime elements are interactive. Some are static text, which we can't render a Tooltip on in an accessible way. We considered having two components, RelativeTime (as-is) and RelativeTimeWithTooltip, but right now consumers wrap the RelativeTime component in a <Link> tag, so it'd be hard to determine in the component what it's parent is and whether or not it should render a Tooltip. Also, consumers may not know the difference between the two. But if you think otherwise, we can explore it.

[siddharthkp]

Ah, I seeee! Thanks you going into detail, now it's very clear to me.

The tricky part is that not all RelativeTime elements are interactive.

I completely missed this detail!

---

Summarising for myself first:

Current scenario:

• RelativeTime renders an inline text element (usually short text like "3 days ago")
• We add a title attribute to the element which has the full datetime, but is only available to visual users, not to screen reader users

Ideal scenario:

• Case 1: If RelativeTime (inline text element) is wrapped in an interactive element like Link, the parent element can receive focus and we can add a Tooltip on the parent element with full datetime. To make sure there isn't also a redundant browser tooltip, we need to remove title attribute
• Case 2: If RelativeTime is standing on it's own without an interactive parent, there is no way to show full datetime consistently between visual users and screen reader users. We need to modify the visual text to show more details. We could keep the title attribute, but it's better to nudge developers to keep the experience consistent

[lindseywild]

@siddharthkp Everything you said looks good, one slight modification:

"We add a title attribute to the element which has the full datetime, but is only available to visual users, not to screen reader users or keyboard or mobile users"

And regarding:

We could keep the title attribute, but it's better to nudge developers to keep the experience consistent

Correct! Eventually we hope to have a better solution, but this is just one small step to getting there :)

[siddharthkp]

I am also thinking that if audit issues start coming in, similar to the one linked in the PR, it gives teams a way to fix the audit issue with minimal impact

Very fair, happy to merge this as a temporary "unblocker" for teams.

We can think of this as a "baby step" to a fully accessible solution. Since we don't have one right now, this is better than what currently exists, but still not fully ideal.

Just wanted to note that this kind of change is scary because even though I called it a temporary unblocker, once it's in dotcom, it's not temporary anymore. To remove it from primer, we would need to create a plan to remove it in dotcom first. I'd nudge you to follow up with a discussion post where we can try to find a long term fix.

[siddharthkp]

Approved. See also: #4635 (comment)

Ready to merge whenever you'd like

[lindseywild]

@siddharthkp Thank you! That totally makes sense. That plan would also need to include a deprecation plan for relative-time-element since it's reliant on that... I can follow up with a Discussion post with the current findings and research we've done :)

[lindseywild]

@siddharthkp posted! #4645