fix(a11y): replace composer auto-focus with header focus in RoomView#7336
fix(a11y): replace composer auto-focus with header focus in RoomView#7336OtavioStasiak wants to merge 4 commits into
Conversation
WalkthroughReplaces imperative ref-based focus management in RoomHeader with declarative KeyboardFocusView props controlled by a new ChangesAccessibility Focus Management Refactor
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/containers/RoomHeader/RoomHeader.tsx (1)
88-88: 💤 Low valueRedundant type alias.
Since the ref-wrapped variant was removed,
IRoomHeaderPropsis now just an alias forIRoomHeader. Consider renamingIRoomHeaderdirectly toIRoomHeaderProps(or dropping the alias and usingIRoomHeader) to remove the indirection.Proposed cleanup
-interface IRoomHeader { +interface IRoomHeaderProps { title?: string; ... } -type IRoomHeaderProps = IRoomHeader; - const SubTitle = ...🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/containers/RoomHeader/RoomHeader.tsx` at line 88, IRoomHeaderProps is a redundant alias for IRoomHeader; remove the indirection by either renaming the interface IRoomHeader to IRoomHeaderProps or deleting the alias and using IRoomHeader directly. Update the declaration in RoomHeader.tsx (replace the type alias line `type IRoomHeaderProps = IRoomHeader;`) and then update any references to the old symbol across the file (and imports/usages elsewhere) to use the chosen canonical name (IRoomHeaderProps or IRoomHeader) so types remain consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@app/containers/RoomHeader/RoomHeader.tsx`:
- Line 88: IRoomHeaderProps is a redundant alias for IRoomHeader; remove the
indirection by either renaming the interface IRoomHeader to IRoomHeaderProps or
deleting the alias and using IRoomHeader directly. Update the declaration in
RoomHeader.tsx (replace the type alias line `type IRoomHeaderProps =
IRoomHeader;`) and then update any references to the old symbol across the file
(and imports/usages elsewhere) to use the chosen canonical name
(IRoomHeaderProps or IRoomHeader) so types remain consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8fdf516c-fc9a-443e-8a4d-1f5dad62f14f
⛔ Files ignored due to path filters (1)
app/containers/RoomHeader/__snapshots__/RoomHeader.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (4)
app/containers/RoomHeader/RoomHeader.tsxapp/containers/RoomHeader/index.tsxapp/lib/hooks/useIsAccessibilityNavigationEnabled.tsapp/views/RoomView/index.tsx
📜 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). (1)
- GitHub Check: ESLint and Test / run-eslint-and-test
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use descriptive names for functions, variables, and classes that clearly convey their purpose
Write comments that explain the 'why' behind code decisions, not the 'what'
Keep functions small and focused on a single responsibility
Use const by default, let when reassignment is needed, and avoid var
Prefer async/await over .then() chains for handling asynchronous operations
Use explicit error handling with try/catch blocks for async operations
Avoid deeply nested code; refactor complex logic into helper functions
Files:
app/lib/hooks/useIsAccessibilityNavigationEnabled.tsapp/containers/RoomHeader/RoomHeader.tsxapp/containers/RoomHeader/index.tsxapp/views/RoomView/index.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript for type safety; add explicit type annotations to function parameters and return types
Prefer interfaces over type aliases for defining object shapes in TypeScript
Use enums for sets of related constants rather than magic strings or numbersUse TypeScript with strict mode and baseUrl set to app/ for import resolution
Files:
app/lib/hooks/useIsAccessibilityNavigationEnabled.tsapp/containers/RoomHeader/RoomHeader.tsxapp/containers/RoomHeader/index.tsxapp/views/RoomView/index.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Use Prettier with tabs, single quotes, 130 char width, no trailing commas, arrow parens avoid, bracket same line
Use@rocket.chat/eslint-configbase with React, React Native, TypeScript, Jest plugins
Files:
app/lib/hooks/useIsAccessibilityNavigationEnabled.tsapp/containers/RoomHeader/RoomHeader.tsxapp/containers/RoomHeader/index.tsxapp/views/RoomView/index.tsx
app/containers/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Reusable UI components should be placed in app/containers/ directory
Files:
app/containers/RoomHeader/RoomHeader.tsxapp/containers/RoomHeader/index.tsx
app/views/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
View components (70+ screen components) should be placed in app/views/ directory
Files:
app/views/RoomView/index.tsx
🧠 Learnings (1)
📚 Learning: 2026-04-30T17:07:51.020Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7274
File: app/lib/services/voip/MediaCallEvents.ts:0-0
Timestamp: 2026-04-30T17:07:51.020Z
Learning: In this Rocket.Chat React Native codebase, the ESLint rule `no-void: error` is enforced. When you see a promise returned from an async call that is not awaited (a “floating promise”), do not silence it with the `void somePromise()` pattern. Instead, handle the promise explicitly by attaching `.catch(...)` (or otherwise awaiting/handling the error) so unhandled-rejection risks are addressed in a way that satisfies the existing ESLint configuration.
Applied to files:
app/lib/hooks/useIsAccessibilityNavigationEnabled.tsapp/containers/RoomHeader/RoomHeader.tsxapp/containers/RoomHeader/index.tsxapp/views/RoomView/index.tsx
🔇 Additional comments (4)
app/containers/RoomHeader/index.tsx (1)
6-44: LGTM!app/views/RoomView/index.tsx (1)
32-32: LGTM!Also applies to: 141-141, 254-256, 378-380
app/lib/hooks/useIsAccessibilityNavigationEnabled.ts (1)
9-12: ⚡ Quick winRequest original review content to rewrite: the
<review_comment>text wasn’t provided here, so the comment can’t be rewritten. Send the<review_comment>...</review_comment>block.app/containers/RoomHeader/RoomHeader.tsx (1)
206-236: 🏗️ Heavy liftMissing input: please provide the original review comment inside a
<review_comment>tag so I can rewrite it according to the required format.
Proposed changes
Replace composer auto-focus with header focus in RoomView.
Issue(s)
https://rocketchat.atlassian.net/browse/NATIVE-1125
How to test or reproduce
Screenshots
Types of changes
Checklist
Further comments
Summary by CodeRabbit