-
Notifications
You must be signed in to change notification settings - Fork 370
Description
Describe the problem
Currently the <HelperTextItem>
has a screen reader element in it that has a few issues. Currently the message there feels a bit broken since many will not know about it and what the message says by default. There is also no way to disable it as when you provide an empty screenReaderText
prop it will instead render : ;
which will be read out to screenreaders and cause confusion.
To us this is restrictive and makes an issue for our test suite and translation tooling. Translating the status message means another value for us to track that isn't needed, requires mapping of the status, and can create issues with some languages where the screenreader text would make more sense in the beginning or simply be in a completely separate element.
How do you reproduce the problem?
See https://www.patternfly.org/components/helper-text
patternfly-react/packages/react-core/src/components/HelperText/HelperTextItem.tsx
Lines 53 to 70 in debf928
screenReaderText = `${variant} status`, | |
...props | |
}: HelperTextItemProps) => { | |
const Component = component as any; | |
const isNotDefaultVariant = variant !== 'default'; | |
const defaultIcon = isNotDefaultVariant && defaultVariantIcons[variant]; | |
return ( | |
<Component | |
className={css(styles.helperTextItem, isNotDefaultVariant && styles.modifiers[variant], className)} | |
id={id} | |
{...props} | |
> | |
{(defaultIcon || icon) && <span className={css(styles.helperTextItemIcon)}>{icon || defaultIcon}</span>} | |
<span className={css(styles.helperTextItemText)}> | |
{children} | |
{isNotDefaultVariant && <span className="pf-v6-screen-reader">: {screenReaderText};</span>} | |
</span> |
Expected behavior
Few options I can expect
- The screenreader text is included in an aria attribute instead
- Screenreader text removed completely
screenReaderText
can be forbidden to render through a prop or empty prop
I would prefer first or second in the list as the latter means we need to change our codebase a bit
Is this issue blocking you?
Yes. It breaks our tests from PF5 to PF6 and our text already covers what the issue type is. Indicating the status in a screen reader that I can't turn off messes up with screen readers.
- PF6 migration PR: Migration to Patternfly v6 cockpit-project/cockpit#21611
- Example failing test
test/verify/check-shell-keys TestKeys.testPrivateKeys
here: https://cockpit-logs.us-east-1.linodeobjects.com/pull-21611-5857ef38-20250314-100948-fedora-41-other/log.html#71
What is your product and what release date are you targeting?
Cockpit. Currently migrating to PF6 from PF5 and want it completed by March 28th as we are in a merge and release freeze
Any other information?
Metadata
Metadata
Assignees
Labels
Type
Projects
Status