Skip to content

Conversation

@ergot-rp
Copy link
Contributor

@ergot-rp ergot-rp commented Oct 3, 2025

For WCAG compability, a fieldset must have a legend. The FieldGroup-component used in the profile-page returns a fieldset. To assure WCAG-compability a visually hidden fieldset is added

WA-64

Summary by CodeRabbit

  • New Features

    • Improved accessibility in the Accessibility settings by adding a visually hidden legend to the Adjustable layout controls, enhancing screen reader clarity without changing the visual UI.
  • Chores

    • Added a changelog entry and version bump to reflect the accessibility fix.

@ergot-rp ergot-rp requested review from a team as code owners October 3, 2025 13:16
@dionisio-bot
Copy link
Contributor

dionisio-bot bot commented Oct 3, 2025

Looks like this PR is ready to merge! 🎉
If you have any trouble, please check the PR guidelines

@changeset-bot
Copy link

changeset-bot bot commented Oct 3, 2025

🦋 Changeset detected

Latest commit: 5c14897

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

This PR includes changesets to release 41 packages
Name Type
@rocket.chat/meteor Minor
@rocket.chat/core-typings Minor
@rocket.chat/rest-typings Minor
@rocket.chat/uikit-playground Patch
@rocket.chat/api-client Patch
@rocket.chat/apps Patch
@rocket.chat/core-services Patch
@rocket.chat/cron Patch
@rocket.chat/ddp-client Patch
@rocket.chat/freeswitch Patch
@rocket.chat/fuselage-ui-kit Major
@rocket.chat/gazzodown Major
@rocket.chat/http-router Patch
@rocket.chat/livechat Patch
@rocket.chat/model-typings Patch
@rocket.chat/ui-avatar Major
@rocket.chat/ui-client Major
@rocket.chat/ui-contexts Major
@rocket.chat/web-ui-registration Major
@rocket.chat/account-service Patch
@rocket.chat/authorization-service Patch
@rocket.chat/ddp-streamer Patch
@rocket.chat/omnichannel-transcript Patch
@rocket.chat/presence-service Patch
@rocket.chat/queue-worker Patch
@rocket.chat/stream-hub-service Patch
@rocket.chat/federation-matrix Patch
@rocket.chat/license Patch
@rocket.chat/media-calls Patch
@rocket.chat/omnichannel-services Patch
@rocket.chat/pdf-worker Patch
@rocket.chat/presence Patch
rocketchat-services Patch
@rocket.chat/models Patch
@rocket.chat/network-broker Patch
@rocket.chat/omni-core-ee Patch
@rocket.chat/mock-providers Patch
@rocket.chat/ui-video-conf Major
@rocket.chat/ui-voip Major
@rocket.chat/instance-status Patch
@rocket.chat/omni-core Patch

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

@CLAassistant
Copy link

CLAassistant commented Oct 3, 2025

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
2 out of 3 committers have signed the CLA.

✅ dougfabris
✅ rodrigok
❌ ergotse
You have signed the CLA already but the status is still pending? Let us recheck it.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 3, 2025

Walkthrough

Adds a visually hidden legend for a fieldset in the AccessibilityPage to satisfy WCAG by importing and using VisuallyHidden from react-aria; also adds a changeset updating package version and changelog entry.

Changes

Cohort / File(s) Summary
Accessibility enhancement
apps/meteor/client/views/account/accessibility/AccessibilityPage.tsx
Imported VisuallyHidden from react-aria and wrapped an existing legend in a visually hidden context inside the Adjustable_layout section; no exported API changes.
Changelog / release
.changeset/ten-roses-brake.md
New changeset bumping @rocket.chat/meteor minor version with entry: "Adds missing legend for fieldset in accessibility page to meet WCAG compliance."

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Areas to check:

  • AccessibilityPage.tsx: confirm legend placement and that visually hidden semantics are correct for screen readers.
  • Changeset content and version bump correctness.

Suggested reviewers

  • scuciatto

Poem

I hop through forms with quiet grace,
A hidden legend finds its place.
Soft as moss, yet clear as day,
Screen readers lead the gentle way.
Ears perked high, I nudge the code—accessibility on the road. 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: adding a missing FieldGroup legend in the accessibility page to improve accessibility compliance.
Linked Issues check ✅ Passed The PR implements the requirement from WA-64 by adding a visually hidden legend to the fieldset in the AccessibilityPage component to meet WCAG compliance.
Out of Scope Changes check ✅ Passed All changes are scoped to the objective: adding VisuallyHidden legend to FieldGroup in AccessibilityPage and updating the changeset documentation accordingly.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between de961e6 and 1013ef1.

📒 Files selected for processing (1)
  • .changeset/ten-roses-brake.md (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: aleksandernsilva
Repo: RocketChat/Rocket.Chat PR: 36974
File: apps/meteor/client/components/Omnichannel/OutboundMessage/components/OutboundMessageWizard/forms/MessageForm/MessageForm.tsx:124-129
Timestamp: 2025-09-18T17:32:33.969Z
Learning: The ARIA mismatch issue in MessageForm's template field (where FieldError id used templateId instead of messageFormId) was addressed in PR #36972 through refactoring the template field into a separate TemplateField component, which uses consistent templateFieldId for both aria-describedby and FieldError id.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: 📦 Build Packages
  • GitHub Check: CodeQL-Build
🔇 Additional comments (1)
.changeset/ten-roses-brake.md (1)

1-5: Changeset is well-formed and properly describes the accessibility improvement.

The changeset correctly bumps the package to a minor version and provides a clear changelog entry that aligns with the PR objectives to add a visually hidden legend for WCAG compliance. The format follows the standard changesets convention.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
apps/meteor/client/views/account/accessibility/AccessibilityPage.tsx (1)

138-140: Consider screen reader user experience with duplicate context.

The legend text t('Adjustable_layout') duplicates the accordion item title on line 136. Screen reader users will hear this label twice in quick succession:

  1. When navigating to the accordion item ("Adjustable_layout")
  2. When entering the fieldset ("Adjustable_layout")

While technically correct for WCAG compliance, consider one of these alternatives for better UX:

  1. Use a more descriptive legend that adds context, e.g., t('Adjustable_layout_preferences') or t('Adjustable_layout_settings').
  2. Keep the current text if the duplication is acceptable in your UX testing.

You can validate the screen reader experience using tools like NVDA, JAWS, or VoiceOver to ensure the repetition isn't jarring.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between ae632f7 and de961e6.

⛔ Files ignored due to path filters (1)
  • packages/apps-engine/deno-runtime/deno.lock is excluded by !**/*.lock
📒 Files selected for processing (1)
  • apps/meteor/client/views/account/accessibility/AccessibilityPage.tsx (2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: 📦 Build Packages
  • GitHub Check: Builds matrix rust bindings against alpine
  • GitHub Check: CodeQL-Build
🔇 Additional comments (2)
apps/meteor/client/views/account/accessibility/AccessibilityPage.tsx (2)

138-140: Verify FieldGroup legend support and avoid redundant announcements

  • Ensure @rocket.chat/fuselage’s FieldGroup renders a native <fieldset> and accepts a <legend> child without layout or accessibility regressions.
  • Consider whether using the same text as the accordion title causes redundant announcements for screen reader users or if a more descriptive legend is needed.

23-23: Verify VisuallyHidden export and consider stable react-aria version
Confirm that VisuallyHidden is correctly exported by your pre-release react-aria dependency and consider upgrading to the latest stable release (e.g., 3.43.2) to reduce risk from nightly builds.

Copy link
Member

@d-gubert d-gubert left a comment

Choose a reason for hiding this comment

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

Please revert the deno.lock file. It seems out of scope for your PR and looks like it was generated by a different version of Deno

@dougfabris dougfabris changed the title fix: (1.3.1) Added a visually hidden legends in FieldGroup fix(a11y): Missing FieldGroup legend in profile page Nov 11, 2025
@dougfabris dougfabris added this to the 7.13.0 milestone Nov 11, 2025
@dougfabris dougfabris changed the title fix(a11y): Missing FieldGroup legend in profile page fix(a11y): Missing FieldGroup legend in accessibility page Nov 11, 2025
dougfabris
dougfabris previously approved these changes Nov 11, 2025
@dougfabris dougfabris dismissed stale reviews from d-gubert and MartinSchoeler November 11, 2025 18:36

stale

@dougfabris dougfabris changed the title fix(a11y): Missing FieldGroup legend in accessibility page feat(a11y): Missing FieldGroup legend in accessibility page Nov 11, 2025
@dougfabris dougfabris added the stat: QA assured Means it has been tested and approved by a company insider label Nov 11, 2025
@dionisio-bot dionisio-bot bot added the stat: ready to merge PR tested and approved waiting for merge label Nov 11, 2025
@kodiakhq kodiakhq bot merged commit 67fa491 into RocketChat:develop Nov 13, 2025
74 of 82 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stat: QA assured Means it has been tested and approved by a company insider stat: ready to merge PR tested and approved waiting for merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants