Skip to content

ROX-33675: Use screenReaderText prop for empty Th elements#19521

Merged
pedrottimark merged 3 commits intomasterfrom
ROX-33675-screenReaderText
Mar 21, 2026
Merged

ROX-33675: Use screenReaderText prop for empty Th elements#19521
pedrottimark merged 3 commits intomasterfrom
ROX-33675-screenReaderText

Conversation

@pedrottimark
Copy link
Contributor

@pedrottimark pedrottimark commented Mar 20, 2026 •
edited
Loading

Description

Replace work-around for accessibility with prop supported in PatternFly 5.3 that I failed to do before PatternFly 6 upgrade. Better late than never.

patternfly/patternfly-react#10152

https://www.patternfly.org/components/table#th

screenReaderText Visually hidden text accessible only via assistive technologies. This must be passed in if the th is intended to be visually empty, and must be conveyed as a column header text.

Analysis

Find in Files pf-v6-screen-reader

  • in cypress folder has 5 results in 5 files

    Selectors are valid because work-around rendered same HTML as Th element with screenReaderText prop.

  • in src folder has 37 results in 32 files

  • in eslint-plugins folder has 3 results in 2 files:

    • pluginAccessibility.js file has 'Th-screenReaderText' rule for screenReaderText prop.

    • pluginPatternFly.js file has 'Td-dataLabel-Th-text' rule with work-around and commented-out code for screenReaderText prop.

Solution

Use typescript-eslint playground for rule to verify replacement of work-arounnd.

https://typescript-eslint.io/play/#ts=5.9.3&showAST=es&fileType=.tsx

  1. Edit pluginPatternFly.js file.

    • Replace work-around with commented-out assignment of hasValueAsScreenReaderText condition in 'Td-dataLabel-Th-text' rule.

    • Add 'no-span-pf-v6-screen-reader' rule.

  2. Replace work-around in src files.

User-facing documentation

  • CHANGELOG.md update is not needed
  • documentation PR is not needed

Testing and quality

  • the change is production ready: the change is GA, or otherwise the functionality is gated by a feature flag
  • CI results are inspected

Automated testing

  • added and edited lint rule
  • added unit tests
  • added e2e tests
  • added regression tests
  • added compatibility tests
  • modified existing tests

How I validated my change

  1. npm run tsc in ui/apps/platform folder.
  2. npm run lint:fast-dev in ui/apps/platform folder.
    Before changes, see presence of errors.
    After changes, see absence of errors.
  3. npm run start in ui/apps/platform folder with staging demo as central.

Manual testing

  1. Visit /main/vulnerabilities/workload-cves

    Before and after changes, see markup in Elements pane of browser:

    <th tabindex="-1" scope="col" class="pf-v6-c-table__th">
    <span class="pf-v6-screen-reader">Row actions</span>
</th>

@pedrottimark pedrottimark requested a review from a team as a code owner March 20, 2026 14:03
@rhacs-bot
Copy link
Contributor

rhacs-bot commented Mar 20, 2026 •
edited
Loading

Images are ready for the commit at b9a90c5.

To use with deploy scripts, first export MAIN_IMAGE_TAG=4.11.x-394-gb9a90c56df.

@codecov
Copy link

codecov bot commented Mar 20, 2026 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 49.24%. Comparing base (19e5460) to head (b9a90c5).
⚠️ Report is 8 commits behind head on master.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master   #19521      +/-   ##
==========================================
+ Coverage   49.23%   49.24%   +0.01%     
==========================================
  Files        2727     2727              
  Lines      205764   205787      +23     
==========================================
+ Hits       101306   101350      +44     
+ Misses      96920    96902      -18     
+ Partials     7538     7535       -3
Flag Coverage Δ
go-unit-tests 49.24% <ø> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

dvail
dvail approved these changes Mar 20, 2026
View reviewed changes
Copy link
Contributor

@dvail dvail left a comment

Choose a reason for hiding this comment

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

Great idea, thanks Mark!

@openshift-ci
Copy link

openshift-ci bot commented Mar 20, 2026

@pedrottimark: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/ocp-4-21-ui-e2e-tests b9a90c5 link false /test ocp-4-21-ui-e2e-tests

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@pedrottimark pedrottimark merged commit bff9977 into master Mar 21, 2026
102 of 104 checks passed
@pedrottimark pedrottimark deleted the ROX-33675-screenReaderText branch March 21, 2026 01:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dvail dvail dvail approved these changes

Assignees

No one assigned

Labels

area/ui

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@pedrottimark @rhacs-bot @dvail