Skip to content

feat(a11y): support message keyboard long press#7206

Merged
OtavioStasiak merged 6 commits into
developfrom
feat.a11y-on-long-keyboard-press
Apr 23, 2026
Merged

feat(a11y): support message keyboard long press#7206
OtavioStasiak merged 6 commits into
developfrom
feat.a11y-on-long-keyboard-press

Conversation

@OtavioStasiak

@OtavioStasiak OtavioStasiak commented Apr 22, 2026

Copy link
Copy Markdown
Contributor

Proposed changes

on long key press on message it must open the actionSheet.

Issue(s)

https://rocketchat.atlassian.net/browse/MA-276

How to test or reproduce

  • Connect Keyboard;
  • navigate to RoomView;
  • Focus on a message;
  • LongPress Enter;

Screenshots

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • Improvement (non-breaking change which improves a current function)
  • New feature (non-breaking change which adds functionality)
  • Documentation update (if none of the other choices apply)

Checklist

  • I have read the CONTRIBUTING doc
  • I have signed the CLA
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works (if applicable)
  • I have added necessary documentation (if applicable)
  • Any dependent changes have been merged and published in downstream modules

Further comments

Summary by CodeRabbit

  • New Features

    • Improved keyboard focus handling for interactive touch controls.
    • Added support for external keyboard navigation.
  • Bug Fixes

    • Fixed focus and disabled-state behavior to ensure consistent focusability and interaction when toggling enabled/disabled.
  • Tests

    • Login automation updated to advance focus (press Enter) before password entry.

@OtavioStasiak OtavioStasiak marked this pull request as ready for review April 22, 2026 17:21
@coderabbitai

coderabbitai Bot commented Apr 22, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7ddd3017-8890-4745-b7f1-582a9162186a

📥 Commits

Reviewing files that changed from the base of the PR and between 8136f77 and 76c470b.

📒 Files selected for processing (1)
  • .maestro/helpers/login.yaml
📜 Recent 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 (2)
📓 Common learnings
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6970
File: .maestro/tests/room/share-message.yaml:77-79
Timestamp: 2026-03-17T19:15:30.463Z
Learning: In `.maestro/tests/room/share-message.yaml` (Rocket.Chat React Native), the `tapOn: point: 5%,10%` step is intentional: it taps the empty area above the bottom sheet and keyboard to dismiss both simultaneously. Using `action-sheet-handle` instead would only close the sheet but not the keyboard. This pattern is acceptable when both need to be dismissed together in a single step.
📚 Learning: 2026-03-17T19:15:30.463Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6970
File: .maestro/tests/room/share-message.yaml:77-79
Timestamp: 2026-03-17T19:15:30.463Z
Learning: In `.maestro/tests/room/share-message.yaml` (Rocket.Chat React Native), the `tapOn: point: 5%,10%` step is intentional: it taps the empty area above the bottom sheet and keyboard to dismiss both simultaneously. Using `action-sheet-handle` instead would only close the sheet but not the keyboard. This pattern is acceptable when both need to be dismissed together in a single step.

Applied to files:

  • .maestro/helpers/login.yaml
🔇 Additional comments (1)
.maestro/helpers/login.yaml (1)

21-26: Clarify the purpose of pressKey: enter in this shared login helper.

This step appears redundant: pressing Enter on the username field (which has returnKeyType='next' and onSubmitEditing={() => setFocus('password')} in UserForm.tsx) will focus the password field automatically, making the subsequent tapOn: id: login-view-password unnecessary.

Additionally, modifying a shared login helper used by many flows warrants clarification about:

  1. Whether this step is actually required for its intended purpose (a11y/keyboard navigation testing, or something else), and
  2. Whether the redundancy is intentional or an oversight.

If this is meant to validate keyboard navigation, consider whether it belongs in the shared login helper or in a dedicated accessibility test file to avoid coupling unrelated flows to this behavior.


Walkthrough

Wraps platform touchable with withKeyboardFocus, updates focus/disabled props, adds react-native-external-keyboard dependency, and inserts an enter keypress step in the Maestro login flow.

Changes

Cohort / File(s) Summary
Keyboard Focus Integration
app/containers/message/Touch.tsx
Wraps the platform-specific touchable in withKeyboardFocus (KeyboardComponent = withKeyboardFocus(Component)) and renders <KeyboardComponent>; moves disabled={!enabled} to after ...props and sets focusable={enabled} and canBeFocused={enabled}.
Dependencies
package.json
Adds dependency react-native-external-keyboard at ^0.9.0.
E2E/Login Flow
.maestro/helpers/login.yaml
Inserts a pressKey: enter step after entering the username/email and before tapping the password field to advance focus/state.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested labels

type: feature

🚥 Pre-merge checks | ✅ 5
✅ 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 'feat(a11y): support message keyboard long press' accurately describes the main change: adding keyboard long-press support for messages to improve accessibility.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ 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.

❤️ Share

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
app/containers/message/Touch.tsx (1)

36-49: ⚠️ Potential issue | 🟠 Major

Preserve disabled when deriving keyboard focusability.

The disabled prop from ...props is overwritten by disabled={!enabled} on line 92, breaking callers like Message.tsx that pass disabled={true}. When enabled defaults to true, a disabled Touch component remains pressable and keyboard-focusable unless callers also explicitly set enabled={false}.

🐛 Proposed fix
 		{
 			children,
 			onPress,
 			android_rippleColor,
@@
 			style,
 			rectButtonStyle,
 			enabled = true,
+			disabled,
 			...props
 		},
 		ref
 	) => {
 		const { colors } = useTheme();
+		const isDisabled = disabled || !enabled;
@@
 				{...touchableProps}
 				{...props}
-				disabled={!enabled}
-				focusable={enabled}
-				canBeFocused={enabled}>
+				disabled={isDisabled}
+				focusable={!isDisabled}
+				canBeFocused={!isDisabled}>

Also applies to: 86-94

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/containers/message/Touch.tsx` around lines 36 - 49, The Touch component
currently overwrites any incoming disabled prop by setting disabled = !enabled;
instead derive an effectiveDisabled that preserves an explicit prop: read
disabled from ...props (e.g., const propDisabled = props.disabled) and compute
effectiveDisabled = typeof propDisabled === 'boolean' ? propDisabled : !enabled,
then use effectiveDisabled wherever the component currently uses disabled
(keyboard focusability, accessibility props and press handling). Update the same
logic blocks around the previous disabled assignment (the code paths referenced
in the comment) so callers like Message.tsx that pass disabled={true} are
respected.
🧹 Nitpick comments (1)
app/containers/message/Touch.tsx (1)

86-94: Put margin styles on the keyboard wrapper.

withKeyboardFocus adds a wrapper; the library documents style for the inner component and containerStyle for the wrapper. Keeping marginStyles in style can make the keyboard focus container bounds diverge from the visual/touch target. See the upstream README for withKeyboardFocus: https://github.com/ArturKalach/react-native-external-keyboard#withkeyboardfocus---for-pressable-touchable-and-more

♻️ Proposed adjustment
 			<KeyboardComponent
 				ref={ref}
 				onPress={onPress}
-				style={[rectButtonStyle, marginStyles, { backgroundColor, borderRadius }]}
+				containerStyle={marginStyles}
+				style={[rectButtonStyle, { backgroundColor, borderRadius }]}
 				{...touchableProps}
 				{...props}
 				disabled={!enabled}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/containers/message/Touch.tsx` around lines 86 - 94, The marginStyles are
applied to the inner KeyboardComponent style which misplaces the keyboard-focus
wrapper; update the JSX for KeyboardComponent so that marginStyles is removed
from the style array passed to the inner component (the existing style prop that
contains rectButtonStyle and { backgroundColor, borderRadius }) and instead pass
marginStyles to the wrapper via containerStyle (e.g.,
containerStyle={marginStyles} or merged with any existing containerStyle); keep
all other props (onPress, disabled, focusable, canBeFocused, touchableProps,
props) unchanged and ensure withKeyboardFocus still receives the containerStyle
so the visual/touch bounds match the focusable wrapper.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@app/containers/message/Touch.tsx`:
- Around line 36-49: The Touch component currently overwrites any incoming
disabled prop by setting disabled = !enabled; instead derive an
effectiveDisabled that preserves an explicit prop: read disabled from ...props
(e.g., const propDisabled = props.disabled) and compute effectiveDisabled =
typeof propDisabled === 'boolean' ? propDisabled : !enabled, then use
effectiveDisabled wherever the component currently uses disabled (keyboard
focusability, accessibility props and press handling). Update the same logic
blocks around the previous disabled assignment (the code paths referenced in the
comment) so callers like Message.tsx that pass disabled={true} are respected.

---

Nitpick comments:
In `@app/containers/message/Touch.tsx`:
- Around line 86-94: The marginStyles are applied to the inner KeyboardComponent
style which misplaces the keyboard-focus wrapper; update the JSX for
KeyboardComponent so that marginStyles is removed from the style array passed to
the inner component (the existing style prop that contains rectButtonStyle and {
backgroundColor, borderRadius }) and instead pass marginStyles to the wrapper
via containerStyle (e.g., containerStyle={marginStyles} or merged with any
existing containerStyle); keep all other props (onPress, disabled, focusable,
canBeFocused, touchableProps, props) unchanged and ensure withKeyboardFocus
still receives the containerStyle so the visual/touch bounds match the focusable
wrapper.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 17f0cb39-3005-44e9-b683-e2423f4f3f41

📥 Commits

Reviewing files that changed from the base of the PR and between 9d43512 and 194931b.

⛔ Files ignored due to path filters (2)
  • ios/Podfile.lock is excluded by !**/*.lock
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (2)
  • app/containers/message/Touch.tsx
  • package.json
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,jsx,ts,tsx,json}

📄 CodeRabbit inference engine (CLAUDE.md)

Configure Prettier with tabs, single quotes, 130 character width, no trailing commas, arrow parens avoid, and bracket same line

Files:

  • package.json
  • app/containers/message/Touch.tsx
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Use ESLint with @rocket.chat/eslint-config base configuration including React, React Native, TypeScript, and Jest plugins

Files:

  • app/containers/message/Touch.tsx
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Use TypeScript with strict mode enabled and configure baseUrl to app/ for import resolution

**/*.{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 numbers

Files:

  • app/containers/message/Touch.tsx
app/containers/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Place reusable UI components in app/containers/ directory

Files:

  • app/containers/message/Touch.tsx
**/*.{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/containers/message/Touch.tsx
🧠 Learnings (11)
📓 Common learnings
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6970
File: .maestro/tests/room/share-message.yaml:77-79
Timestamp: 2026-03-17T19:15:30.463Z
Learning: In `.maestro/tests/room/share-message.yaml` (Rocket.Chat React Native), the `tapOn: point: 5%,10%` step is intentional: it taps the empty area above the bottom sheet and keyboard to dismiss both simultaneously. Using `action-sheet-handle` instead would only close the sheet but not the keyboard. This pattern is acceptable when both need to be dismissed together in a single step.
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.
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-07T17:49:17.538Z
Learning: Support React 19, React Native 0.79, and Expo 53
📚 Learning: 2026-04-07T17:49:17.538Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-07T17:49:17.538Z
Learning: Support React 19, React Native 0.79, and Expo 53

Applied to files:

  • package.json
📚 Learning: 2026-02-05T13:55:00.974Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6930
File: package.json:101-101
Timestamp: 2026-02-05T13:55:00.974Z
Learning: In this repository, the dependency on react-native-image-crop-picker should reference the RocketChat fork (RocketChat/react-native-image-crop-picker) with explicit commit pins, not the upstream ivpusic/react-native-image-crop-picker. Update package.json dependencies (and any lockfile) to point to the fork URL and a specific commit, ensuring edge-to-edge Android fixes are included. This pattern should apply to all package.json files in the repo that declare this dependency.

Applied to files:

  • package.json
📚 Learning: 2026-03-31T11:59:31.061Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6875
File: android/build.gradle:3-8
Timestamp: 2026-03-31T11:59:31.061Z
Learning: In the RocketChat/Rocket.Chat.ReactNative repository, the React Native upgrade helper (https://react-native-community.github.io/upgrade-helper/?from=0.79.4&to=0.81.5) recommends kotlinVersion = "2.1.20", compileSdkVersion = 36, targetSdkVersion = 36, and buildToolsVersion = "36.0.0" in android/build.gradle for the RN 0.79.4 → 0.81.5 upgrade. These are the sanctioned values for this upgrade path and should not be flagged as compatibility concerns.

Applied to files:

  • package.json
📚 Learning: 2026-03-31T11:58:54.608Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6875
File: android/gradle/wrapper/gradle-wrapper.properties:3-3
Timestamp: 2026-03-31T11:58:54.608Z
Learning: In the RocketChat/Rocket.Chat.ReactNative repository, the React Native upgrade helper (https://react-native-community.github.io/upgrade-helper/) recommends gradle-8.14.3-bin for the React Native 0.79.4 → 0.81.5 upgrade. This is the sanctioned Gradle version for RN 0.81.5 even though it is above Kotlin 2.1.20's "fully supported" Gradle range (≤8.11); Kotlin 2.1.20 docs explicitly allow using newer Gradle versions with a caveat that deprecation warnings may appear.

Applied to files:

  • package.json
📚 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/containers/message/Touch.tsx
📚 Learning: 2026-04-07T17:49:17.538Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-07T17:49:17.538Z
Learning: Applies to app/AppContainer.tsx : Implement root navigation container logic in app/AppContainer.tsx to switch between authentication states

Applied to files:

  • app/containers/message/Touch.tsx
📚 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/containers/message/Touch.tsx
📚 Learning: 2026-04-07T17:49:17.538Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-07T17:49:17.538Z
Learning: Applies to app/containers/**/*.{ts,tsx} : Place reusable UI components in app/containers/ directory

Applied to files:

  • app/containers/message/Touch.tsx
📚 Learning: 2026-04-07T17:49:17.538Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-07T17:49:17.538Z
Learning: Applies to app/index.tsx : Configure Redux provider, theme, navigation, and notifications in app/index.tsx

Applied to files:

  • app/containers/message/Touch.tsx
📚 Learning: 2026-04-07T17:49:17.538Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-07T17:49:17.538Z
Learning: Applies to app/theme.tsx : Configure theming context in app/theme.tsx

Applied to files:

  • app/containers/message/Touch.tsx

@Rohit3523 Rohit3523 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@OtavioStasiak OtavioStasiak temporarily deployed to approve_e2e_testing April 23, 2026 21:37 — with GitHub Actions Inactive
@OtavioStasiak OtavioStasiak had a problem deploying to experimental_android_build April 23, 2026 21:42 — with GitHub Actions Failure
@OtavioStasiak OtavioStasiak had a problem deploying to official_android_build April 23, 2026 21:42 — with GitHub Actions Failure
@OtavioStasiak OtavioStasiak had a problem deploying to experimental_ios_build April 23, 2026 21:42 — with GitHub Actions Failure
@OtavioStasiak OtavioStasiak merged commit 53c3364 into develop Apr 23, 2026
29 of 39 checks passed
@OtavioStasiak OtavioStasiak deleted the feat.a11y-on-long-keyboard-press branch April 23, 2026 23:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants