Skip to content

Conversation

@sfc-gh-bnisco
Copy link
Collaborator

@sfc-gh-bnisco sfc-gh-bnisco commented Dec 18, 2025

Describe your changes

  • Created a new ESLint rule no-aria-hidden-with-focusable-children to prevent hiding interactive elements from screen readers.
  • Enabled stricter accessibility linting rules:
    • jsx-a11y/control-has-associated-label - Requires accessible names for icon-only controls
    • jsx-a11y/no-aria-hidden-on-focusable - Prevents hiding focusable elements from assistive technology
    • jsx-a11y/no-noninteractive-tabindex - Avoids making non-interactive elements keyboard-focusable
  • Fixes an existing violation of these rules.

Testing Plan

  • The new ESLint rule includes comprehensive test cases covering various scenarios of proper and improper aria-hidden usage.
  • The rule is now enabled in our ESLint configuration, which is run in CI.

Contribution License Agreement

By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.

@github-actions
Copy link
Contributor

github-actions bot commented Dec 18, 2025

✅ PR preview is ready!

Name Link
📦 Wheel file https://core-previews.s3-us-west-2.amazonaws.com/pr-13412/streamlit-1.52.2-py3-none-any.whl
📦 @streamlit/component-v2-lib Download from artifacts
🕹️ Preview app pr-13412.streamlit.app (☁️ Deploy here if not accessible)

Copy link
Collaborator Author

sfc-gh-bnisco commented Dec 18, 2025

@snyk-io
Copy link
Contributor

snyk-io bot commented Dec 18, 2025

Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@sfc-gh-bnisco sfc-gh-bnisco added change:feature PR contains new feature or enhancement implementation impact:users PR changes affect end users security-assessment-completed Security assessment has been completed for PR labels Dec 18, 2025 — with Graphite App
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR enhances accessibility linting by introducing a custom ESLint rule to prevent hiding interactive elements from screen readers, enabling three stricter jsx-a11y rules from the ecosystem, and fixing an existing violation where a checkbox lacked an accessible label.

Key Changes

  • Created no-aria-hidden-with-focusable-children custom ESLint rule to detect when aria-hidden="true" is applied to elements containing focusable descendants
  • Enabled jsx-a11y/control-has-associated-label, jsx-a11y/no-aria-hidden-on-focusable, and jsx-a11y/no-noninteractive-tabindex rules
  • Added aria-label to the audio recording checkbox in ScreencastDialog

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
frontend/eslint.config.mjs Enables the new custom rule and three stricter jsx-a11y rules in the ESLint configuration
frontend/eslint-plugin-streamlit-custom/src/no-aria-hidden-with-focusable-children.ts Implements the custom ESLint rule that recursively checks for focusable descendants within aria-hidden containers
frontend/eslint-plugin-streamlit-custom/src/no-aria-hidden-with-focusable-children.test.ts Adds comprehensive test cases covering valid and invalid usage patterns of the new rule
frontend/eslint-plugin-streamlit-custom/src/index.ts Registers the new rule in the plugin's exports
frontend/eslint-plugin-streamlit-custom/src/index.test.ts Updates the test to verify the new rule is properly exported
frontend/app/src/hocs/withScreencast/components/ScreencastDialog/ScreencastDialog.tsx Fixes a11y violation by adding aria-label to the checkbox control

@github-actions
Copy link
Contributor

github-actions bot commented Dec 18, 2025

📉 Frontend coverage change detected

The frontend unit test (vitest) coverage has decreased by 0.0000%

  • Current PR: 86.4200% (12694 lines, 1723 missed)
  • Latest develop: 86.4200% (12694 lines, 1723 missed)

✅ Coverage change is within normal range.

📊 View detailed coverage comparison

@sfc-gh-bnisco sfc-gh-bnisco force-pushed the 12-18-_feat_update_agents_rules_for_better_a11y_practices branch from 74c62a2 to 8b8a1f9 Compare December 18, 2025 18:19
@sfc-gh-bnisco sfc-gh-bnisco force-pushed the 12-18-_feat_update_eslint_rules_for_better_a11y_support branch from dfe8c9d to 99c90d7 Compare December 18, 2025 18:19
@sfc-gh-bnisco sfc-gh-bnisco marked this pull request as ready for review December 19, 2025 00:07
Copy link
Collaborator

@lukasmasuch lukasmasuch left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Copy link
Collaborator Author

sfc-gh-bnisco commented Dec 22, 2025

Merge activity

  • Dec 22, 5:24 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Dec 22, 6:03 PM UTC: Graphite rebased this pull request as part of a merge.
  • Dec 22, 6:19 PM UTC: @sfc-gh-bnisco merged this pull request with Graphite.

@sfc-gh-bnisco sfc-gh-bnisco changed the base branch from 12-18-_feat_update_agents_rules_for_better_a11y_practices to graphite-base/13412 December 22, 2025 17:41
@sfc-gh-bnisco sfc-gh-bnisco changed the base branch from graphite-base/13412 to develop December 22, 2025 18:01
@sfc-gh-bnisco sfc-gh-bnisco requested a review from a team as a code owner December 22, 2025 18:01
@sfc-gh-bnisco sfc-gh-bnisco force-pushed the 12-18-_feat_update_eslint_rules_for_better_a11y_support branch from 99c90d7 to e7f33a2 Compare December 22, 2025 18:03
@sfc-gh-bnisco sfc-gh-bnisco merged commit ab3c508 into develop Dec 22, 2025
42 checks passed
@sfc-gh-bnisco sfc-gh-bnisco deleted the 12-18-_feat_update_eslint_rules_for_better_a11y_support branch December 22, 2025 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

change:feature PR contains new feature or enhancement implementation impact:users PR changes affect end users security-assessment-completed Security assessment has been completed for PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants