Skip to content

fix: reset RoomItem press highlight after leaving a room#7464

Open
diegolmello wants to merge 1 commit into
developfrom
indecisive-viburnum
Open

fix: reset RoomItem press highlight after leaving a room#7464
diegolmello wants to merge 1 commit into
developfrom
indecisive-viburnum

Conversation

@diegolmello

@diegolmello diegolmello commented Jul 2, 2026

Copy link
Copy Markdown
Member

Proposed changes

The room row in the rooms list kept its pressed highlight after opening a room and navigating back — the row stayed drawn with the grey press color indefinitely.

The cause is a legacy RectButton (the old react-native-gesture-handler imperative API) nested inside a new GestureDetector. The two gesture-arbitration systems do not cooperate, so the button's press-out event — the one that clears the highlight — was dropped when navigation fired synchronously on press. The highlight was set on touch-down, but its reset never arrived.

This removes RectButton from RoomItem and drives the press highlight from a Gesture.Tap placed in the same Gesture.Race tree as the existing pan (swipe) and long-press gestures. The highlight is cleared in .onFinalize, which runs on the UI thread for every terminal outcome — success, cancel, or interruption by navigation — so it can no longer get stuck.

Only app/containers/RoomItem/Touchable.tsx changes behaviourally; the RoomItem snapshot is regenerated to reflect the swapped touchable subtree.

Issue(s)

https://rocketchat.atlassian.net/browse/NATIVE-1380

How to test or reproduce

  1. Open the rooms list.
  2. Tap any room to open it, then navigate back.
  3. Before this change the tapped row stays highlighted grey; after it, the row returns to its normal background.

Also confirm swipe-to-reveal (favorite / hide) and long-press still work on a row.

Screenshots

Verified on an Android emulator (API 36). The tapped row background, sampled at native resolution, reads (255, 255, 255) both before tapping and after returning to the list; a stuck press state would read approximately (228, 231, 234). Tap navigation and swipe-to-reveal actions both continue to work.

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

The snapshot diff is large because the RoomItem touchable subtree is serialized across every test case; the change is the RectButton to Gesture.Tap swap repeated per case, not a change to the rows themselves.

The transient press-appearance could not be captured on video: adb screenrecord on the software-rendered emulator truncated to under a second of static frames. That was never the defect, though — the reported bug was the highlight sticking, which the before/after row sampling above disproves.

Summary by CodeRabbit

  • New Features

    • Improved room item interaction with clearer tap feedback and smoother touch handling.
    • Added better keyboard focus support for room items.
  • Bug Fixes

    • Made press and release behavior more reliable during touch gestures.
    • Ensured long-press and drag interactions continue to work as expected.

The room row kept its pressed highlight after entering a room and
navigating back. A legacy RectButton (old gesture-handler API) was
nested inside a new GestureDetector; the two arbitration systems do
not cooperate, so the button's press-out (which clears the highlight)
was dropped when navigation fired synchronously on press.

Drop RectButton and drive the highlight from a Gesture.Tap in the same
Gesture.Race tree as pan and long-press. Clear it in .onFinalize, which
runs on the UI thread for every terminal outcome (success, cancel, or
interrupted-by-navigation), so it can no longer get stuck.
@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

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: c752d70a-f257-47a3-b79d-e87b0f0beacb

📥 Commits

Reviewing files that changed from the base of the PR and between 20720c6 and fdc5b94.

⛔ Files ignored due to path filters (1)
  • app/containers/RoomItem/__snapshots__/RoomItem.test.tsx.snap is excluded by !**/*.snap
📒 Files selected for processing (1)
  • app/containers/RoomItem/Touchable.tsx
📜 Recent review details
🧰 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/containers/RoomItem/Touchable.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 numbers

Use TypeScript with strict mode enabled

Files:

  • app/containers/RoomItem/Touchable.tsx
**/*.{js,jsx,ts,tsx,json}

📄 CodeRabbit inference engine (CLAUDE.md)

Use Prettier formatting with tabs, single quotes, 130 character line width, no trailing commas, and avoid arrow function parentheses

Files:

  • app/containers/RoomItem/Touchable.tsx
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Enforce ESLint rules from @rocket.chat/eslint-config with React, React Native, TypeScript, and Jest plugins

Files:

  • app/containers/RoomItem/Touchable.tsx
app/containers/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Place reusable UI components in 'app/containers/' directory

Files:

  • app/containers/RoomItem/Touchable.tsx
🧠 Learnings (2)
📚 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/containers/RoomItem/Touchable.tsx
📚 Learning: 2026-06-25T18:37:44.793Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7434
File: app/views/ScreenLockConfigView.tsx:101-141
Timestamp: 2026-06-25T18:37:44.793Z
Learning: In the Rocket.Chat React Native codebase, do not treat passing an `async` function directly to an event prop in React/React Native UI components (e.g., `onPress={async () => ...}` in TSX) as a “floating promises” CI-blocking lint issue—this repo does not enable the ESLint `no-floating-promises` rule (while `no-void` is enforced). Only raise robustness follow-ups when there are genuinely unhandled promise paths (e.g., fire-and-forget calls like `save()` that return a Promise that is neither awaited nor handled), and prefer making sure failure paths are explicitly handled/reported rather than blocking on lint-style floating-promise concerns.

Applied to files:

  • app/containers/RoomItem/Touchable.tsx
🔇 Additional comments (4)
app/containers/RoomItem/Touchable.tsx (4)

2-10: LGTM!

Also applies to: 41-41


192-207: Tap gesture correctly clears pressed on .onFinalize.

Using onFinalize (fires on success, cancel, and failure) instead of only onEnd correctly addresses the stuck-highlight bug described in the PR. Logic here looks sound.


209-220: LGTM! pressAnimatedStyle correctly interpolates background color off pressed and isFocused.


192-207: 🎯 Functional Correctness

Remove the double-fire concern. KeyboardFocusView uses onPress/onLongPress for keyboard/remote handling via useKeyboardPress; it does not forward them as touch handlers. The tap/long-press gestures are the touch path here, so a single tap should not invoke handlePress twice.

			> Likely an incorrect or invalid review comment.

Walkthrough

Touchable.tsx replaces the previous Touch component with KeyboardFocusView and reanimated-based press styling. A new tap gesture is added alongside existing pan and long-press gestures, driving a pressed shared value that interpolates background color via a new pressAnimatedStyle.

Changes

Touchable Press-State Animation

Layer / File(s) Summary
Imports and shared state setup
app/containers/RoomItem/Touchable.tsx
Adds reanimated interpolateColor and gesture imports, KeyboardFocusView, and a new pressed shared value for tracking tap state.
Tap gesture and press animation logic
app/containers/RoomItem/Touchable.tsx
Adds a Tap gesture toggling pressed and scheduling handlePress, races it with pan and long-press gestures, and defines pressAnimatedStyle interpolating background color from pressed/isFocused.
Render tree wiring
app/containers/RoomItem/Touchable.tsx
Replaces the Touch wrapper with KeyboardFocusView and an Animated.View using pressAnimatedStyle, wiring onPress/onLongPress/testID to existing handlers.

Estimated code review effort: 2 (Simple) | ~15 minutes

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant TapGesture
    participant PressedValue as pressed (shared value)
    participant PressAnimatedStyle
    participant Touchable as Touchable Component

    User->>TapGesture: onBegin (touch down)
    TapGesture->>PressedValue: set pressed = true
    PressedValue->>PressAnimatedStyle: interpolate background color
    PressAnimatedStyle->>Touchable: update Animated.View style
    User->>TapGesture: onEnd (release, success)
    TapGesture->>PressedValue: set pressed = false
    TapGesture->>Touchable: schedule handlePress
    PressedValue->>PressAnimatedStyle: interpolate background color
    PressAnimatedStyle->>Touchable: update Animated.View style
Loading

Suggested labels: type: bug

🚥 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 clearly describes the RoomItem press-highlight bug fix after leaving a room.
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.

Warning

Review ran into problems

🔥 Problems

Errors were encountered while retrieving linked issues.

Errors (1)
  • NATIVE-1380: Request failed with status code 401

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.

@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown

@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown

iOS Build Available

Rocket.Chat 4.75.0.109276

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.

1 participant