fix: delete background taller than its row on ServersHistory - #7536
fix: delete background taller than its row on ServersHistory#7536OtavioStasiak wants to merge 5 commits into
Conversation
GitHub Actions
Error
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📜 Recent review details⏰ Context from checks skipped due to timeout. (3)
WalkthroughThe swipe-to-delete components remove the unused ChangesSwipe delete layout cleanup
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (4)
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 |
GitHub Actions
Error
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/containers/ServerItem/SwipeableDeleteItem/Actions.tsx (1)
28-33: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd explicit return types to the changed functions.
DeleteActionis exported, but its callback passed tomemorelies on return-type inference.triggerDeleteAnimationalso relies on inference. Add an explicit component return type, such asReactElement, and add: voidtotriggerDeleteAnimation.As per coding guidelines, add explicit type annotations to function parameters and return types.
🤖 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/ServerItem/SwipeableDeleteItem/Actions.tsx` around lines 28 - 33, Add explicit return types to the functions in DeleteAction: annotate the memoized component callback with the appropriate ReactElement return type and annotate triggerDeleteAnimation with : void, while preserving the existing behavior and parameter types.Source: Coding guidelines
🤖 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.
Inline comments:
In `@app/containers/ServerItem/SwipeableDeleteItem/Actions.tsx`:
- Line 34: Update the haptic call in the delete action within Actions.tsx to
attach a catch handler to Haptics.impactAsync, preventing native failures from
becoming unhandled promise rejections while keeping the animation non-blocking.
---
Nitpick comments:
In `@app/containers/ServerItem/SwipeableDeleteItem/Actions.tsx`:
- Around line 28-33: Add explicit return types to the functions in DeleteAction:
annotate the memoized component callback with the appropriate ReactElement
return type and annotate triggerDeleteAnimation with : void, while preserving
the existing behavior and parameter types.
🪄 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 Plus
Run ID: ec578b44-27d5-420f-992c-242bec633012
⛔ Files ignored due to path filters (2)
app/containers/ServerItem/__snapshots__/ServerItem.test.tsx.snapis excluded by!**/*.snapapp/views/NewServerView/components/ServersHistoryItem/__snapshots__/ServersHistoryItem.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (3)
app/containers/ServerItem/SwipeableDeleteItem/Actions.tsxapp/containers/ServerItem/SwipeableDeleteItem/Touchable.tsxapp/containers/ServerItem/Touchable.tsx
💤 Files with no reviewable changes (1)
- app/containers/ServerItem/SwipeableDeleteItem/Touchable.tsx
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: ESLint and Test / run-eslint-and-test
- GitHub Check: format
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{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/ServerItem/Touchable.tsxapp/containers/ServerItem/SwipeableDeleteItem/Actions.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
Files:
app/containers/ServerItem/Touchable.tsxapp/containers/ServerItem/SwipeableDeleteItem/Actions.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{js,jsx,ts,tsx}: Before committing changes to JavaScript or TypeScript files, runpnpm prettier-lintandTZ=UTC pnpm testfor the modified files.
Use the local-first data flow: the UI reads from WatermelonDB, while sagas synchronize data with the server.
Use Redux and Redux-Saga for global or server state, and use Zustand for feature-local stores; do not assume all state is in Redux.
Files:
app/containers/ServerItem/Touchable.tsxapp/containers/ServerItem/SwipeableDeleteItem/Actions.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/ServerItem/Touchable.tsxapp/containers/ServerItem/SwipeableDeleteItem/Actions.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/ServerItem/Touchable.tsxapp/containers/ServerItem/SwipeableDeleteItem/Actions.tsx
🔇 Additional comments (2)
app/containers/ServerItem/SwipeableDeleteItem/Actions.tsx (1)
22-26: LGTM!Also applies to: 35-118, 120-136
app/containers/ServerItem/Touchable.tsx (1)
6-6: LGTM!Also applies to: 34-46
OtavioStasiak
changed the title
GitHub Actions
Inactive
GitHub Actions
Error
OtavioStasiak
changed the title
OtavioStasiak
changed the title
GitHub Actions
Inactive
GitHub Actions
Error
GitHub Actions
Error
GitHub Actions
Error
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
GitHub Actions
Inactive
GitHub Actions
Error
GitHub Actions
Error
GitHub Actions
Error
GitHub Actions
Error
GitHub Actions
Inactive
GitHub Actions
Waiting
GitHub Actions
Waiting
3 participants
Proposed changes
The workspaces action sheet renders a red bar under the last row. This isn't a background color—it's the swipe-to-delete action showing through.
DeleteActionrenders a full-width red plate behind every server row. Previously, the plate height was calculated asrowHeight + SERVER_ITEM_PADDING_VERTICALto account for the row's vertical padding. This worked whileROW_HEIGHT = 56, because the rendered row height was44 + 12 + 12 = 68, matching the plate.PR #7449 changed
ROW_HEIGHTto68(the actual rendered height), added an explicit row height, and removedminHeight. Although the row still renders at68dp, the constant now already includes the padding, whileActions.tsxcontinued addingSERVER_ITEM_PADDING_VERTICAL, making the plate80dptall. As a result,12dpof the delete action became visible beneath the last row.The issue only appears on the final row because intermediate rows are covered by the next sibling and separator. The last row ends at the sheet's safe-area padding, leaving the overflow exposed.
This fix removes the height arithmetic entirely and stretches the plate using
top: 0andbottom: 0, allowing it to always match the parent's height regardless ofROW_HEIGHTorfontScale. As part of this cleanup,SERVER_ITEM_PADDING_VERTICALand the unusedrowHeightprop were removed fromActions.tsx,SwipeableDeleteItem/Touchable.tsx, andServerItem/Touchable.tsx.Additionally, a
.catch()was added to the floatingHaptics.impactAsync()promise without awaiting it, preserving the gesture timing while avoiding an unhandled promise.No row heights change. The only layout difference is replacing
"height": 80withtop: 0/bottom: 0; the68dprow height introduced in #7449 remains unchanged, preserving pixel-grid alignment. This also fixes a secondary issue where, at largerfontScalevalues (~1.18+), the delete action no longer failed to fill the entire row.Post-mortem
b4e58d724(2026-07-01) exists only ondevelopand would have shipped in4.75.0.4.74.0still usesROW_HEIGHT = 56withminHeightand does not exhibit the issue.The root cause was a hidden coupling:
Actions.tsxassumedROW_HEIGHTexcluded vertical padding, but that assumption was never encoded in the implementation. When #7449 legitimately redefined the constant, the dependency remained invisible because the rendered row height did not change.RoomItem/Actions.tsxavoids this problem by deriving its height fromuseResponsiveLayout().rowHeight. Replacing explicit height calculations withtop: 0/bottom: 0removes the coupling entirely and prevents this class of bug in the future.Issue(s)
https://rocketchat.atlassian.net/browse/NATIVE-1446
How to test or reproduce
Android Portrait
Android Landscape
iOS Portrait
iOS Landscape
iPad Portrait
iPad Landscape
Screenshots
Types of changes
Checklist
Further comments
Summary by CodeRabbit
Bug Fixes