Skip to content
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

MiddleTextTruncation: preserve original text in accessibility tree #2339

Merged
merged 8 commits into from
Nov 14, 2024

Conversation

mayank99
Copy link
Contributor

@mayank99 mayank99 commented Nov 14, 2024

Changes

This PR improves the accessibility of the MiddleTextTruncation component. This component is meant for visual truncation and should not affect the accessibility tree because it would make the text confusing and nonsensical. While truncation in general should be avoided for even better accessibility, this is at least better than before.

The behavior of MiddleTextTruncation is now consistent with the browser's built-in text-overflow: ellipsis mechanism, so the same set of UX guidelines could be applied in both cases to tackle the truncation problem. Right now, there is no such UX pattern documented in the design system, but it's becoming clear that there needs to be one.

It might be easier to review the first three commits separately:

  1. 27d4deb: Code reorganization only; no outward behavior changes.
  2. 399ed24: This is the most interesting part. Uses shadow DOM to inject the untruncated text while keeping it visually hidden. The truncated text is removed from the accessibility tree using aria-hidden.
    • 75b8148: I needed to add pointer-events: none, because otherwise the shadow-tree was blocking clicks (problematic when <MiddleTextTruncation> is used inside an interactive element).
  3. 301885b: Removes title attribute from stories and examples. title is not a cure for bad design and should not be shown in any code examples.

Testing

Updated e2e tests and added a new one to verify the new behavior.

Manually tested that the full text is part of the accessibility tree.

Before:

Accessible name for combobox is MyFileWithAReallyLongName…2.html

After:

Accessible name for combobox is "MyFileWithAReallyLongNameThatWillBeTruncatedBecauseItIsReallyThatLongSoHardToBelieve_FinalVersion_V2.html"

Note: With this change, the information in the accessibility tree can become very verbose, which could be potentially annoying for screen-reader users. But I think this is the safer default (and consistent with default overflow behavior), because otherwise there would be no way for the user to access the full text. "verbosity vs content loss" is a topic I would expect to be covered by any truncation UX pattern.

Docs

Added patch changeset.

I also think we should mention this behavior on the documentation page (#2329).

@mayank99 mayank99 self-assigned this Nov 14, 2024
@mayank99 mayank99 marked this pull request as ready for review November 14, 2024 16:22
@mayank99 mayank99 requested a review from a team as a code owner November 14, 2024 16:22
@mayank99 mayank99 requested review from r100-stack and smmr-dn and removed request for a team November 14, 2024 16:22
@mayank99 mayank99 force-pushed the mayank/middletextprops branch from 3b7190e to 75b8148 Compare November 14, 2024 16:36
Copy link
Contributor

@smmr-dn smmr-dn left a comment

Choose a reason for hiding this comment

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

LGTM!

@mayank99 mayank99 merged commit bbf5613 into main Nov 14, 2024
18 checks passed
@mayank99 mayank99 deleted the mayank/middletextprops branch November 14, 2024 22:19
@imodeljs-admin imodeljs-admin mentioned this pull request Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants