feat(voip): Support a11y#7106
Conversation
diegolmello
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
diegolmello
changed the title
diegolmello
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds VoIP accessibility and landscape layout support: new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CallView
participant Responsive as useResponsiveLayout()
participant ScreenReader as useIsScreenReaderEnabled()
participant Store as CallStore
User->>CallView: mount/render
CallView->>Responsive: request width,height
Responsive-->>CallView: return dimensions
CallView->>ScreenReader: query screen-reader state
ScreenReader-->>CallView: return boolean
CallView->>Store: read controlsVisible
Store-->>CallView: return flag
CallView->>CallView: derive effectiveControlsVisible = controlsVisible || screenReaderEnabled
CallView->>User: render UI with landscape layout and accessibility metadata
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
diegolmello
had a problem deploying
to
experimental_android_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
official_ios_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
diegolmello
requested a deployment
to
official_android_build
GitHub Actions
Waiting
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (4)
CLAUDE.md (1)
17-17: Make PR base-branch guidance non-absoluteLine 17 hard-codes
develop, but active work can target other integration branches (as in this PR). Recommend wording this as a default with explicit exceptions to prevent incorrect PR targeting.Proposed doc tweak
-- **PRs:** target `develop` +- **PRs:** target `develop` by default; if the feature is being integrated through a staging branch (e.g., `feat.*`), target that branch as specified by the initiative.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CLAUDE.md` at line 17, Change the hard-coded PR base-branch guidance that reads "**PRs:** target `develop`" to a non-absolute phrasing that treats `develop` as the default (e.g., "PRs: target `develop` by default; use the branch specified in the relevant issue or team guidelines when different") and add a short note about exceptions and where to find the correct target (issue, release notes, or team lead). Update the line containing "**PRs:** target `develop`" accordingly and add a one-sentence pointer to the location or process for exceptions.app/i18n/locales/sl-SI.json (1)
725-725: Consider refining Slovenian phrasing for naturalness.Line 725’s
Preklopi upravljalce klicanjais understandable but sounds unnatural. A more idiomatic equivalent (e.g., “Preklopi kontrolnike klica”) would likely read better in UI and screen readers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/i18n/locales/sl-SI.json` at line 725, The Slovenian string for the key Toggle_call_controls is phrased unnaturally; update its value from "Preklopi upravljalce klicanja" to a more idiomatic form such as "Preklopi kontrolnike klica" so the UI and screen readers read naturally while keeping the same key name Toggle_call_controls.UBIQUITOUS_LANGUAGE.md (1)
26-27: Clarify “UI Kit” naming to avoid framework ambiguity.Line 26 says “UIKit interactive element,” which reads like Apple UIKit. Prefer “UI Kit block element” to match Rocket.Chat terminology and avoid confusion in this glossary.
Suggested wording
-| **Block** | A UIKit interactive element rendered within a message (buttons, selects, overflow menus) | UI block, interactive element, action block | +| **Block** | A UI Kit interactive element rendered within a message (buttons, selects, overflow menus) | UI block, interactive element, action block |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@UBIQUITOUS_LANGUAGE.md` around lines 26 - 27, Update the glossary entry for "Block" to avoid confusion with Apple UIKit by replacing the phrase "UIKit interactive element" with "UI Kit block element" (or "UI Kit interactive element") and ensure related aliases like "UI block, interactive element, action block" remain consistent; edit the table row that currently reads `| **Block** | A UIKit interactive element rendered within a message (buttons, selects, overflow menus) | UI block, interactive element, action block |` to use the new "UI Kit" wording so it matches Rocket.Chat terminology.app/lib/hooks/useIsScreenReaderEnabled.test.ts (1)
21-25: UsewaitForinstead of emptyasync actfor deterministic async assertion.The empty
await act(async () => {})pattern is less explicit and can become flaky. Other hook tests in this directory already usewaitForfrom@testing-library/react-nativefor this pattern—adopt the same approach here for consistency and determinism.♻️ Proposed refactor
-import { renderHook, act } from '@testing-library/react-native'; +import { renderHook, act, waitFor } from '@testing-library/react-native'; @@ it('returns true after isScreenReaderEnabled resolves true', async () => { jest.spyOn(AccessibilityInfo, 'isScreenReaderEnabled').mockResolvedValue(true); const { result } = renderHook(() => useIsScreenReaderEnabled()); - await act(async () => {}); - expect(result.current).toBe(true); + await waitFor(() => { + expect(result.current).toBe(true); + }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/lib/hooks/useIsScreenReaderEnabled.test.ts` around lines 21 - 25, The test for useIsScreenReaderEnabled uses an empty await act to wait for async state; replace it with waitFor from `@testing-library/react-native` to make the async assertion deterministic. Specifically, keep the jest.spyOn(AccessibilityInfo, 'isScreenReaderEnabled').mockResolvedValue(true) and renderHook(() => useIsScreenReaderEnabled()), then call waitFor(() => expect(result.current).toBe(true)) so the assertion waits for the hook state update instead of using an empty act.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/i18n/locales/fi.json`:
- Line 779: The Finnish translation for the key "Toggle_call_controls" uses the
non-idiomatic word “hallintoja”; update the value for "Toggle_call_controls" in
app/i18n/locales/fi.json to a more natural UI phrasing such as "Vaihda puhelun
ohjaimet" or, if the action explicitly shows/hides controls, "Näytä/piilota
puhelun ohjaimet" so the label reads naturally in the UI.
In `@app/i18n/locales/hu.json`:
- Line 813: The translation for the key "Toggle_call_controls" currently reads
as "Hívásváltási vezérlők" which implies switching rather than toggling; update
the value for "Toggle_call_controls" to a phrase that clearly conveys on/off
toggling (for example: "Hívásvezérlők ki/be kapcsolása" or another concise
Hungarian equivalent) so the label explicitly indicates a toggle action.
In `@app/i18n/locales/pt-PT.json`:
- Line 499: Update the Portuguese (pt-PT) translation value for the
"Toggle_call_controls" key to use the pt-PT term "controlos" instead of the BR
variant "controles"—replace the current value "Alternar controles de chamada"
with "Alternar controlos da chamada" in the localization entry for
"Toggle_call_controls".
In `@app/views/CallView/components/CallButtons.tsx`:
- Around line 64-66: The container rendering the call controls (in
CallView/components/CallButtons.tsx where pointerEvents,
accessibilityElementsHidden and testID are set) doesn't hide hidden controls
from Android TalkBack; add importantForAccessibility with the value
'no-hide-descendants' when controlsVisible is false (and 'auto' or the existing
default when true) so TalkBack will skip the visually-hidden controls; update
the same element that currently sets pointerEvents and
accessibilityElementsHidden to include
importantForAccessibility={controlsVisible ? 'auto' : 'no-hide-descendants'} (or
equivalent string constants) to ensure Android accessibility excludes hidden
controls.
In `@app/views/CallView/index.test.tsx`:
- Around line 39-41: The global mock for the hook useIsScreenReaderEnabled is
using mockReturnValue which persists implementations across tests; change the
default jest.mock implementation to use mockReturnValue(false) for the base mock
but replace any test-specific overrides that currently call
mockReturnValue(true) with mockReturnValueOnce(true) (both in the initial
jest.mock block and the second override later in the file that targets
useIsScreenReaderEnabled) so the true response is consumed only for that single
test and doesn't leak into subsequent tests.
In `@docs/superpowers/plans/2026-04-06-voip-accessibility.md`:
- Around line 730-739: The CallButtons container sets borderTopColor but not
borderLeftColor, so in landscape when styles.buttonsContainerLandscape uses
borderLeftWidth the divider color is invisible; update the Animated.View style
array (the instance rendering CallButtons where
pointerEvents/accessibilityElementsHidden/testID are set) to also apply
borderLeftColor: colors.strokeExtraLight when isLandscape (or add a conditional
style object that sets borderLeftColor when styles.buttonsContainerLandscape is
active) so the landscape divider uses the same stroke color.
In `@docs/superpowers/specs/2026-04-06-voip-accessibility-design.md`:
- Around line 54-62: The doc is inconsistent: update the CallView — landscape
layout section so it does NOT claim "isLandscape is passed as a prop to
CallerInfo and CallButtons"; instead state that CallerInfo and CallButtons each
derive isLandscape locally (e.g., via const { width, height } =
useResponsiveLayout(); const isLandscape = width > height) and that they must
not call useWindowDimensions() directly, keeping useResponsiveLayout() as the
single source of truth; keep the notes about Avatar size change and layout
behavior but remove any instruction to pass isLandscape down from CallView.
---
Nitpick comments:
In `@app/i18n/locales/sl-SI.json`:
- Line 725: The Slovenian string for the key Toggle_call_controls is phrased
unnaturally; update its value from "Preklopi upravljalce klicanja" to a more
idiomatic form such as "Preklopi kontrolnike klica" so the UI and screen readers
read naturally while keeping the same key name Toggle_call_controls.
In `@app/lib/hooks/useIsScreenReaderEnabled.test.ts`:
- Around line 21-25: The test for useIsScreenReaderEnabled uses an empty await
act to wait for async state; replace it with waitFor from
`@testing-library/react-native` to make the async assertion deterministic.
Specifically, keep the jest.spyOn(AccessibilityInfo,
'isScreenReaderEnabled').mockResolvedValue(true) and renderHook(() =>
useIsScreenReaderEnabled()), then call waitFor(() =>
expect(result.current).toBe(true)) so the assertion waits for the hook state
update instead of using an empty act.
In `@CLAUDE.md`:
- Line 17: Change the hard-coded PR base-branch guidance that reads "**PRs:**
target `develop`" to a non-absolute phrasing that treats `develop` as the
default (e.g., "PRs: target `develop` by default; use the branch specified in
the relevant issue or team guidelines when different") and add a short note
about exceptions and where to find the correct target (issue, release notes, or
team lead). Update the line containing "**PRs:** target `develop`" accordingly
and add a one-sentence pointer to the location or process for exceptions.
In `@UBIQUITOUS_LANGUAGE.md`:
- Around line 26-27: Update the glossary entry for "Block" to avoid confusion
with Apple UIKit by replacing the phrase "UIKit interactive element" with "UI
Kit block element" (or "UI Kit interactive element") and ensure related aliases
like "UI block, interactive element, action block" remain consistent; edit the
table row that currently reads `| **Block** | A UIKit interactive element
rendered within a message (buttons, selects, overflow menus) | UI block,
interactive element, action block |` to use the new "UI Kit" wording so it
matches Rocket.Chat terminology.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 29a635f6-8347-441a-983e-8b089b2ce614
⛔ Files ignored due to path filters (5)
app/containers/NewMediaCall/__snapshots__/PeerItem.test.tsx.snapis excluded by!**/*.snapapp/containers/NewMediaCall/__snapshots__/PeerList.test.tsx.snapis excluded by!**/*.snapapp/views/CallView/__snapshots__/index.test.tsx.snapis excluded by!**/*.snapapp/views/CallView/components/Dialpad/__snapshots__/Dialpad.test.tsx.snapis excluded by!**/*.snapapp/views/CallView/components/__snapshots__/CallerInfo.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (42)
.gitignoreCLAUDE.mdUBIQUITOUS_LANGUAGE.mdapp/containers/NewMediaCall/PeerItem.tsxapp/i18n/locales/ar.jsonapp/i18n/locales/bn-IN.jsonapp/i18n/locales/cs.jsonapp/i18n/locales/de.jsonapp/i18n/locales/en.jsonapp/i18n/locales/es.jsonapp/i18n/locales/fi.jsonapp/i18n/locales/fr.jsonapp/i18n/locales/hi-IN.jsonapp/i18n/locales/hu.jsonapp/i18n/locales/it.jsonapp/i18n/locales/ja.jsonapp/i18n/locales/nl.jsonapp/i18n/locales/nn.jsonapp/i18n/locales/no.jsonapp/i18n/locales/pt-BR.jsonapp/i18n/locales/pt-PT.jsonapp/i18n/locales/ru.jsonapp/i18n/locales/sl-SI.jsonapp/i18n/locales/sv.jsonapp/i18n/locales/ta-IN.jsonapp/i18n/locales/te-IN.jsonapp/i18n/locales/tr.jsonapp/i18n/locales/zh-CN.jsonapp/i18n/locales/zh-TW.jsonapp/lib/hooks/useIsScreenReaderEnabled.test.tsapp/lib/hooks/useIsScreenReaderEnabled.tsapp/lib/services/voip/useCallStore.tsapp/views/CallView/CallView.stories.tsxapp/views/CallView/components/CallButtons.test.tsxapp/views/CallView/components/CallButtons.tsxapp/views/CallView/components/CallerInfo.tsxapp/views/CallView/components/Dialpad/DialpadButton.tsxapp/views/CallView/index.test.tsxapp/views/CallView/index.tsxapp/views/CallView/styles.tsdocs/superpowers/plans/2026-04-06-voip-accessibility.mddocs/superpowers/specs/2026-04-06-voip-accessibility-design.md
📜 Review details
⏰ 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: ESLint and Test / run-eslint-and-test
- GitHub Check: format
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: OtavioStasiak
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6499
File: app/containers/ServerItem/index.tsx:34-36
Timestamp: 2025-12-17T15:56:22.578Z
Learning: In the Rocket.Chat React Native codebase, for radio button components on iOS, include the selection state ("Selected"/"Unselected") in the accessibilityLabel instead of using accessibilityState={{ checked: hasCheck }}, because iOS VoiceOver has known issues with accessibilityRole="radio" + accessibilityState that prevent correct state announcement.
Learnt from: divyanshu-patil
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6957
File: ios/RocketChat Watch App/Views/MessageComposerView.swift:37-55
Timestamp: 2026-03-04T20:13:17.288Z
Learning: In the WatchOS app (ios/RocketChat Watch App) for Rocket.Chat React Native, using SwiftUI `Button` inside a `ScrollView` on WatchOS causes accidental message sends because button tap targets can be triggered during scroll gestures. `Text` with `.onTapGesture` is the preferred pattern for tappable items in scroll views on WatchOS. To preserve accessibility, add `.accessibilityAddTraits(.isButton)` and `.accessibilityLabel()` to the `Text` element instead.
📚 Learning: 2025-12-17T15:56:22.578Z
Learnt from: OtavioStasiak
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6499
File: app/containers/ServerItem/index.tsx:34-36
Timestamp: 2025-12-17T15:56:22.578Z
Learning: In the Rocket.Chat React Native codebase, for radio button components on iOS, include the selection state ("Selected"/"Unselected") in the accessibilityLabel instead of using accessibilityState={{ checked: hasCheck }}, because iOS VoiceOver has known issues with accessibilityRole="radio" + accessibilityState that prevent correct state announcement.
Applied to files:
app/views/CallView/components/Dialpad/DialpadButton.tsxapp/containers/NewMediaCall/PeerItem.tsxdocs/superpowers/specs/2026-04-06-voip-accessibility-design.md
📚 Learning: 2026-03-04T20:13:17.288Z
Learnt from: divyanshu-patil
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6957
File: ios/RocketChat Watch App/Views/MessageComposerView.swift:37-55
Timestamp: 2026-03-04T20:13:17.288Z
Learning: In the WatchOS app (ios/RocketChat Watch App) for Rocket.Chat React Native, using SwiftUI `Button` inside a `ScrollView` on WatchOS causes accidental message sends because button tap targets can be triggered during scroll gestures. `Text` with `.onTapGesture` is the preferred pattern for tappable items in scroll views on WatchOS. To preserve accessibility, add `.accessibilityAddTraits(.isButton)` and `.accessibilityLabel()` to the `Text` element instead.
Applied to files:
app/views/CallView/components/Dialpad/DialpadButton.tsxdocs/superpowers/specs/2026-04-06-voip-accessibility-design.md
📚 Learning: 2026-03-10T15:21:45.098Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7046
File: app/containers/InAppNotification/NotifierComponent.stories.tsx:46-75
Timestamp: 2026-03-10T15:21:45.098Z
Learning: In `app/containers/InAppNotification/NotifierComponent.tsx` (React Native, Rocket.Chat), `NotifierComponent` is exported as a Redux-connected component via `connect(mapStateToProps)`. The `isMasterDetail` prop is automatically injected from `state.app.isMasterDetail` and does not need to be passed explicitly at call sites or in Storybook stories that use the default (connected) export.
Applied to files:
app/views/CallView/CallView.stories.tsxapp/views/CallView/index.test.tsx
📚 Learning: 2026-03-15T13:55:42.038Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6911
File: app/containers/markdown/Markdown.stories.tsx:104-104
Timestamp: 2026-03-15T13:55:42.038Z
Learning: In Rocket.Chat React Native, the markdown parser requires a space between the underscore wrapping italic text and a mention sigil (_ mention _ instead of _mention_). Ensure stories and tests that include italic-wrapped mentions follow this form to guarantee proper parsing. Specifically, for files like app/containers/markdown/Markdown.stories.tsx, and any test/content strings that exercise italic-mentions, use the pattern _ mention _ (with spaces) to prevent the mention from being treated as plain text. Validate any test strings or story content accordingly.
Applied to files:
app/views/CallView/CallView.stories.tsx
📚 Learning: 2026-03-30T15:49:30.957Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6875
File: app/containers/RoomItem/Actions.tsx:12-12
Timestamp: 2026-03-30T15:49:30.957Z
Learning: In RocketChat/Rocket.Chat.ReactNative, `react-native-worklets` version 0.6.1 does NOT export a built-in Jest mock (e.g., no `react-native-worklets/lib/module/mock`). The correct Jest mock approach for this version is to add a manual mock in `jest.setup.js`: `jest.mock('react-native-worklets', () => ({ scheduleOnRN: jest.fn((fn, ...args) => fn(...args)) }))`.
Applied to files:
app/views/CallView/index.test.tsx
🔇 Additional comments (48)
.gitignore (1)
86-87: Looks good — ignore rules are appropriate.Ignoring
*.p8and.worktrees/is a sensible safeguard to prevent accidental commits of private keys and local worktree artifacts.app/i18n/locales/ta-IN.json (1)
811-811: Looks good for the new accessibility label key.Line 811 adds the expected
Toggle_call_controlsentry with a valid localized value.app/i18n/locales/fr.json (1)
711-711: LGTM for French localization entry.Line 711 correctly adds
Toggle_call_controlswith an appropriate translation.app/i18n/locales/ja.json (1)
518-518: Correct key/value addition for Japanese locale.Line 518 is consistent with the new accessibility toggle label.
app/i18n/locales/ar.json (1)
587-587: Arabic localization entry looks good.Line 587 correctly introduces
Toggle_call_controlsfor the accessibility label.app/i18n/locales/hi-IN.json (1)
811-811: Key matches usage and localization intent.Line 811 correctly adds
Toggle_call_controls, and it aligns with the call-site lookup inapp/views/CallView/components/CallerInfo.tsx(Line 35).app/i18n/locales/it.json (1)
624-624: Looks good.The new Italian key/value is clear and consistent with the intended toggle action.
app/i18n/locales/zh-CN.json (1)
579-579: Looks good.The zh-CN translation is clear and maps well to the toggle behavior.
app/i18n/locales/nn.json (1)
381-381: Looks good.The Nynorsk translation is accurate for a toggle on/off action.
app/i18n/locales/tr.json (1)
607-607: Looks good.The Turkish translation is concise and correctly expresses toggle behavior.
app/i18n/locales/sv.json (1)
778-778: Looks good.The Swedish translation is semantically correct for an accessibility toggle label.
app/i18n/locales/en.json (1)
911-911: Looks good — key is correctly added and aligned with the CallView accessibility usage.app/i18n/locales/de.json (1)
804-804: LGTM — exact key match and valid German localization for the new accessibility label.app/i18n/locales/no.json (1)
854-854: Looks good — Norwegian translation key/value is consistent with the new CallView toggle label.app/i18n/locales/ru.json (1)
756-756: LGTM — Russian translation entry is correctly wired for the new toggle-call-controls label.app/i18n/locales/es.json (1)
430-430: Looks good — Spanish localization forToggle_call_controlsis correctly added.app/i18n/locales/zh-TW.json (1)
607-607: LGTM — zh-TW translation key is correctly added and aligned with the new call-controls accessibility toggle.app/i18n/locales/nl.json (1)
711-711: Good locale key addition for call-controls accessibility toggle.The new
Toggle_call_controlsentry is correctly added and consistent with the feature intent.app/i18n/locales/cs.json (1)
876-876: Looks good — Czech translation key is in place.This matches the new accessibility toggle string introduced for CallView.
app/views/CallView/components/CallButtons.test.tsx (1)
51-51: Nice test fix for hidden controls state.Using
includeHiddenElements: truehere makes the assertion robust with the new accessibility-hidden behavior.app/i18n/locales/bn-IN.json (1)
811-811: Bangla locale update is correctly wired.
Toggle_call_controlsis added in the expected format.app/i18n/locales/pt-BR.json (1)
885-885: PT-BR translation key addition is good.The new
Toggle_call_controlsstring is correctly added for the accessibility toggle flow.app/views/CallView/index.tsx (1)
9-30: Responsive landscape handling is implemented cleanly.The
useResponsiveLayout()+isLandscapestyle switch andcall-view-containertestID make the layout behavior both clear and testable.app/views/CallView/components/Dialpad/DialpadButton.tsx (1)
28-29: Great accessibility improvement on dialpad action targets.Adding
accessibilityRole='button'and a descriptive label for digit/letters improves screen-reader clarity.app/containers/NewMediaCall/PeerItem.tsx (1)
17-18: Good accessibility metadata on selectable peer items.
accessibilityLabel+accessibilityRole="button"is a solid improvement here and aligns with the interaction model.app/lib/services/voip/useCallStore.ts (1)
311-315: Controls visibility fallback for screen readers looks correct.This preserves control availability for assistive tech even when the visual toggle would otherwise hide them.
app/views/CallView/styles.ts (1)
77-92: Landscape style additions are clean and consistent.The new style keys are well-separated and map clearly to the landscape use-cases in the call UI.
app/lib/hooks/useIsScreenReaderEnabled.ts (1)
4-14: Hook implementation is solid.Initialization, event subscription, and cleanup are correctly structured.
app/views/CallView/components/CallerInfo.tsx (1)
31-39: Screen-reader and landscape handling in CallerInfo looks good.Disabling tap-toggle under screen reader while preserving explicit accessibility metadata is a good tradeoff here.
app/views/CallView/index.test.tsx (1)
418-428: Landscape wrapper test coverage is a strong addition.The style assertion for
flexDirection: 'row'gives good confidence on responsive layout behavior.app/views/CallView/CallView.stories.tsx (1)
117-134: Great addition of a dedicated landscape story.This makes the new responsive behavior explicit and easier to validate in Storybook.
docs/superpowers/plans/2026-04-06-voip-accessibility.md (12)
32-141: LGTM! Excellent TDD approach and comprehensive test coverage.The
useIsScreenReaderEnabledhook implementation is clean and follows React Native best practices. The test suite covers all critical scenarios: initial state, async updates, event-driven changes, and cleanup on unmount. The use ofAccessibilityInfo.isScreenReaderEnabled()and thescreenReaderChangedevent listener is the standard approach for screen reader detection in React Native.
144-209: LGTM! Well-structured i18n task with clear verification.The approach of dispatching a general-purpose agent to handle all 25 translations is efficient. The instructions ensure translation quality by requiring natural language translations (not English copies) and proper alphabetical placement. The verification step (
grep -rl "Toggle_call_controls" app/i18n/locales/ | wc -lexpecting 25) provides a simple sanity check.
212-254: LGTM! Clean integration of screen reader state.The modification to
useControlsVisiblecorrectly ensures controls remain visible when a screen reader is active by returningcontrolsVisible || isScreenReaderEnabled. This is a clean, non-breaking change that improves accessibility without requiring changes to consuming components.
257-376: LGTM! Proper screen reader interaction handling.The CallerInfo implementation correctly disables the tap-to-hide gesture when a screen reader is active by setting
onPress={isScreenReaderEnabled ? undefined : toggleControlsVisible}. Combined with theuseControlsVisiblechange, this ensures controls remain visible and accessible to screen reader users. The addition ofaccessibilityLabelandaccessibilityRole='button'follows React Native accessibility best practices.The test coverage is appropriate, mocking AccessibilityInfo to simulate screen reader state and verifying the toggle is disabled.
379-411: LGTM! Proper accessibility tree management.Adding
accessibilityElementsHidden={!controlsVisible}ensures that when call controls are visually hidden, they are also removed from the accessibility tree. This prevents screen reader users from encountering hidden interactive elements, which is an important accessibility best practice in React Native.
414-448: LGTM! Clear and descriptive accessibility labels.The accessibility additions to DialpadButton are well-implemented. The
accessibilityLabelprovides a compound label that includes both the digit and letters when available (e.g., "2 ABC"), which gives screen reader users the full context. TheaccessibilityRole='button'is appropriate for this interactive Pressable component.
451-489: LGTM! Consistent accessibility implementation.The PeerItem accessibility additions are straightforward and correct, using the existing
item.labelfor theaccessibilityLabeland addingaccessibilityRole='button'. This is consistent with the approach taken for DialpadButton and follows React Native accessibility conventions.
550-571: LGTM! Well-structured landscape styles.The landscape style additions are well-designed:
contentContainerLandscapeusesflexDirection: 'row'for side-by-side layoutcallerInfoContainerLandscapeandbuttonsContainerLandscapeuse flex ratios for responsive sizing- Border transition from top to left for the divider is appropriate for landscape orientation
buttonsRowLandscapeadds vertical spacing between button rows
575-613: LGTM! Proper use of responsive layout context.The CallView implementation correctly uses
useResponsiveLayout()to deriveisLandscape = width > heightlocally, avoiding prop drilling and conflicts with tablet layout detection. This aligns with the architectural decision documented in line 7 of the design spec and ensures consistency with the existing shared responsive context.
619-669: LGTM! Responsive avatar sizing in landscape.The CallerInfo landscape implementation is well-executed:
- Derives
isLandscapelocally usinguseResponsiveLayout()- Conditionally applies
callerInfoContainerLandscapestyle- Reduces avatar size from 120 to 80 in landscape to prevent overlap
- Maintains the screen reader interaction handling across orientations
529-539: LGTM! Effective landscape layout test.The landscape test provides good coverage by:
- Using a custom
LandscapeWrapperwithResponsiveLayoutContext.Providerto simulate landscape dimensions (800×400)- Verifying that
flexDirection: 'row'is applied to the container via testID- Confirming the responsive layout logic works as expected
838-855: LGTM! Comprehensive final verification.The final check section ensures quality by running tests for all modified areas (
CallView|useIsScreenReaderEnabled|PeerItem|DialpadButton) and linting the entire codebase. This provides a good sanity check before considering the implementation complete.docs/superpowers/specs/2026-04-06-voip-accessibility-design.md (5)
9-20: LGTM! Clear problem definition and scope boundaries.The problem statement clearly identifies specific accessibility gaps in the VoIP screens while explicitly noting which components already have adequate accessibility coverage. This provides good context for understanding the implementation scope.
22-50: LGTM! Pragmatic architecture with minimal new abstractions.The approach of filling accessibility gaps prop-by-prop with only one new hook (
useIsScreenReaderEnabled) is appropriately minimal and follows existing codebase patterns. The hook architecture is clean and leverages React Native's built-inAccessibilityInfoAPI correctly.
64-101: LGTM! Component specifications align with implementation.The component-level specifications are well-documented and align with the implementation plan:
- CallerInfo correctly handles screen reader state and adds appropriate accessibility metadata
- CallButtons uses
accessibilityElementsHiddento manage accessibility tree visibility- DialpadButton provides compound labels (digit + letters) for comprehensive screen reader announcements
- PeerItem leverages existing labels with proper role attribution
The conditional font scaling approach (line 89) is pragmatic—applying
fontScaleLimitedonly when testing reveals actual overflow issues.
103-111: LGTM! Pragmatic font scaling strategy.The font scaling approach is well-balanced:
- Leverages React Native's default text scaling behavior (no explicit opt-out)
- Correctly identifies that fixed-size touch targets (buttons, avatars) don't need to scale
- Applies
fontScaleLimiteddefensively only when testing reveals actual layout breakage- Follows the existing
FONT_SCALE_LIMIT = 1.3pattern fromuseResponsiveLayout
113-126: LGTM! Clear i18n requirements and well-defined scope boundaries.The i18n requirements are straightforward (single new key), and the "What is NOT in scope" section is valuable for preventing scope creep. It clearly identifies:
- Components that already have adequate accessibility coverage
- Accessibility features that aren't needed (A11y.Order)
- Features intentionally deferred (announceForAccessibility)
This helps keep the implementation focused and manageable.
diegolmello
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
diegolmello
had a problem deploying
to
experimental_android_build
GitHub Actions
Error
diegolmello
requested a deployment
to
official_ios_build
GitHub Actions
Waiting
diegolmello
had a problem deploying
to
official_android_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
diegolmello
force-pushed
the
feat.voip-a11y
branch
from
9a49e39 to
4ea7fba
Compare
diegolmello
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
diegolmello
had a problem deploying
to
official_android_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
experimental_android_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
official_ios_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
diegolmello
had a problem deploying
to
official_android_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
experimental_android_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
official_ios_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
diegolmello
changed the title
Adds design spec for VoIP screen accessibility (feat.voip-lib-new), documents a11y patterns in CLAUDE.md, and extends UBIQUITOUS_LANGUAGE.md with accessibility terminology.
diegolmello
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
- Remove stray merge conflict marker in CallView.stories.tsx - Add importantForAccessibility='no-hide-descendants' for Android TalkBack - Remove unused useResponsiveLayout import from CallButtons.tsx - Remove unused ResponsiveLayoutContext import from index.test.tsx - Update snapshots to reflect new a11y props
diegolmello
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
diegolmello
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
diegolmello
had a problem deploying
to
official_ios_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
official_android_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
experimental_android_build
GitHub Actions
Error
This comment was marked as resolved.
This comment was marked as resolved.
diegolmello
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
diegolmello
had a problem deploying
to
official_android_build
GitHub Actions
Error
diegolmello
temporarily deployed
to
official_ios_build
GitHub Actions
Inactive
diegolmello
temporarily deployed
to
experimental_android_build
GitHub Actions
Inactive
diegolmello
temporarily deployed
to
experimental_ios_build
GitHub Actions
Inactive
diegolmello
had a problem deploying
to
upload_experimental_android
GitHub Actions
Error
|
Android Build Available Rocket.Chat Experimental 4.72.0.108515 Internal App Sharing: https://play.google.com/apps/test/RQVpXLytHNc/ahAO29uNQSqGgxf6eiPZwut1hfEykrT2OsssGP6V8-5JFkf8UJNYzbXh-2NnmhWxVeICWfi0WaeOyMJ9LYjUEMc9vr |
diegolmello
requested a deployment
to
approve_e2e_testing
GitHub Actions
Waiting
diegolmello
requested a deployment
to
official_ios_build
GitHub Actions
Waiting
diegolmello
requested a deployment
to
experimental_android_build
GitHub Actions
Waiting
diegolmello
requested a deployment
to
experimental_ios_build
GitHub Actions
Waiting
diegolmello
requested a deployment
to
official_android_build
GitHub Actions
Waiting
2 participants
Proposed changes
Adds screen reader (VoiceOver/TalkBack) and landscape layout accessibility support to the VoIP CallView.
useIsScreenReaderEnabledhook — subscribes toAccessibilityInfoto reactively detect when a screen reader is activeaccessibilityElementsHiddento prevent VoiceOver/TalkBack from reaching hidden buttonsaccessibilityLabel+accessibilityRoleonDialpadButtonandPeerItem— makes interactive elements fully discoverableuseResponsiveLayout()andisLandscapeToggle_call_controls— added to all locales for the show/hide controls buttonborderLeftColorapplied correctly for the landscapeCallButtonsdividerIssue(s)
https://rocketchat.atlassian.net/browse/VMUX-66
How to test or reproduce
Screenshots
Types of changes
Checklist
Summary by CodeRabbit
New Features
Accessibility
Tests
Documentation
Chores