Skip to content

fix: render list separators consistently by snapping row heights to the pixel grid - #7449

Merged
diegolmello merged 3 commits into
developfrom
stormy-count
Jul 1, 2026
Merged

fix: render list separators consistently by snapping row heights to the pixel grid#7449
diegolmello merged 3 commits into
developfrom
stormy-count

Conversation

@diegolmello

@diegolmello diegolmello commented Jun 26, 2026

Copy link
Copy Markdown
Member

Proposed changes

Some lists intermittently failed to render the 1px row separator, and others rendered it with non-uniform thickness. Reported on RoomsListView and the MessageActions action sheet, but the defect affected most shared list-row components.

Root cause: when a row's rendered height in DP does not land on the device physical-pixel grid, the hairline separator anti-aliases below visibility on some rows and across multiple physical pixels on others. As cumulative row offsets drift fractionally, each successive separator lands at a different sub-pixel phase, so some vanish and some thicken.

Fix: snap each row container height to the pixel grid with PixelRatio.roundToNearestPixel(height * fontScale), so every separator sits at an integer physical-pixel offset and renders as a crisp, uniform hairline. The snap is a no-op where the height was already integer-px and corrective otherwise, so it is always safe. Applied to every shared list-row component:

  • RoomItem and RoomsListView, via the useResponsiveLayout row-height single source of truth
  • ActionSheet Item, via a new useActionSheetItemHeight hook (covers MessageActions)
  • List.Item, DirectoryItem, UserItem, ServerItem, ServersHistoryItem, DepartmentItemFilter

Also fixes a secondary defect: incorrect getItemLayout scroll-math in SelectServerView, QueueListView, TeamChannelsView and ShareListView. Two of them used the array length as the per-row pixel height, and offsets ignored both pixel-snapping and the condensed display mode. These screens do not call scrollToIndex, so the prior impact was mis-sized scroll indicators rather than broken scrolling.

Issue(s)

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

How to test or reproduce

On a device whose pixel density produces fractional physical-pixel row heights (for example a 2.625x Android device), open lists such as RoomsListView, the MessageActions action sheet, the server list, and Admin/settings lists. Before the fix, separators are missing on some rows and uneven on others. After the fix, every separator renders as a uniform hairline. Verified on-device by measuring a flat separator pitch (RoomsListView 217px, ActionSheet 140px).

Screenshots

Before After
Captura de Tela 2026-07-01 à(s) 14 45 38 Captura de Tela 2026-07-01 à(s) 14 44 50

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 snap uses PixelRatio.get() (device density, constant per device) for grid alignment and multiplies by fontScale so rows still grow with the user's font-size setting. Unit suite is green (1770/1770) with no snapshot regressions beyond the two row components whose explicit height legitimately changed.

https://claude.ai/code/session_01CrrW51BkNx2qDu1PZWgK5y

Summary by CodeRabbit

Summary by CodeRabbit

  • Bug Fixes
    • Improved spacing and row/item sizing across lists and pickers by rounding scaled heights to pixel-perfect values for more consistent text scaling.
    • Updated ActionSheets and bottom sheet content to use a shared responsive item-height calculation, including detent sizing behavior.
    • Refined FlatList scrolling and virtualization by adjusting getItemLayout to use pixel-rounded, display-mode-aware row heights.
    • Updated relevant row height constants (e.g., ServerItem/ServersHistoryItem) and aligned header layout height/centering.

@diegolmello
diegolmello temporarily deployed to approve_e2e_testing June 26, 2026 19:20 — with GitHub Actions Inactive
@coderabbitai

coderabbitai Bot commented Jun 26, 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: 16323b96-dbe5-4093-8f32-b72555fb185b

📥 Commits

Reviewing files that changed from the base of the PR and between 6d8b905 and 463e74e.

⛔ Files ignored due to path filters (2)
  • app/containers/ServerItem/__snapshots__/ServerItem.test.tsx.snap is excluded by !**/*.snap
  • app/views/NewServerView/components/ServersHistoryItem/__snapshots__/ServersHistoryItem.test.tsx.snap is excluded by !**/*.snap
📒 Files selected for processing (19)
  • app/containers/ActionSheet/ActionSheet.tsx
  • app/containers/ActionSheet/BottomSheetContent.tsx
  • app/containers/ActionSheet/Item.tsx
  • app/containers/ActionSheet/useActionSheetDetents.ts
  • app/containers/ActionSheet/useActionSheetItemHeight.ts
  • app/containers/DirectoryItem/index.tsx
  • app/containers/List/ListItem.tsx
  • app/containers/MessageActions/Header.tsx
  • app/containers/ServerItem/index.tsx
  • app/containers/ServerItem/styles.ts
  • app/containers/UserItem.tsx
  • app/ee/omnichannel/views/QueueListView.tsx
  • app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx
  • app/views/CannedResponsesListView/DepartmentFilter/DepartmentItemFilter.tsx
  • app/views/NewServerView/components/ServersHistoryItem/index.tsx
  • app/views/NewServerView/components/ServersHistoryItem/styles.ts
  • app/views/SelectServerView.tsx
  • app/views/ShareListView/index.tsx
  • app/views/TeamChannelsView.tsx
✅ Files skipped from review due to trivial changes (2)
  • app/containers/ActionSheet/useActionSheetItemHeight.ts
  • app/containers/ServerItem/styles.ts
🚧 Files skipped from review as they are similar to previous changes (17)
  • app/views/NewServerView/components/ServersHistoryItem/styles.ts
  • app/containers/ActionSheet/ActionSheet.tsx
  • app/containers/List/ListItem.tsx
  • app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx
  • app/containers/ServerItem/index.tsx
  • app/views/ShareListView/index.tsx
  • app/containers/DirectoryItem/index.tsx
  • app/containers/MessageActions/Header.tsx
  • app/containers/ActionSheet/BottomSheetContent.tsx
  • app/containers/ActionSheet/useActionSheetDetents.ts
  • app/views/TeamChannelsView.tsx
  • app/views/CannedResponsesListView/DepartmentFilter/DepartmentItemFilter.tsx
  • app/containers/UserItem.tsx
  • app/containers/ActionSheet/Item.tsx
  • app/ee/omnichannel/views/QueueListView.tsx
  • app/views/SelectServerView.tsx
  • app/views/NewServerView/components/ServersHistoryItem/index.tsx

Walkthrough

ActionSheet sizing now uses a shared item-height hook. Several item rows and list views now compute heights from font scale with pixel rounding, FlatList getItemLayout calculations were updated to use responsive values, and the MessageActions header now sets an explicit container height.

Changes

ActionSheet item height sharing

Layer / File(s) Summary
Shared item height hook
app/containers/ActionSheet/useActionSheetItemHeight.ts
A new hook exports the base ActionSheet item height and rounds it with the current font scale.
ActionSheet consumers
app/containers/ActionSheet/*
ActionSheet, BottomSheetContent, Item, and useActionSheetDetents now read height from the shared hook.

Responsive row height rounding

Layer / File(s) Summary
Rounded responsive layout values
app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx
The shared responsive layout hook rounds base row heights before exposing them.
Item and container heights
app/containers/DirectoryItem/index.tsx, app/containers/List/ListItem.tsx, app/containers/ServerItem/*, app/containers/UserItem.tsx, app/views/CannedResponsesListView/DepartmentFilter/DepartmentItemFilter.tsx, app/views/NewServerView/components/ServersHistoryItem/*
DirectoryItem, ListItem, ServerItem, UserItem, DepartmentItemFilter, and ServersHistoryItem now apply rounded font-scale-based heights, with updated row-height constants where shown.
FlatList item layouts
app/ee/omnichannel/views/QueueListView.tsx, app/views/SelectServerView.tsx, app/views/ShareListView/index.tsx, app/views/TeamChannelsView.tsx
QueueListView, SelectServerView, ShareListView, and TeamChannelsView recalculate getItemLayout using rounded responsive heights and display-mode-aware row sizes.
MessageActions header layout
app/containers/MessageActions/Header.tsx
Header container styling now sets an explicit height and vertical centering.

Estimated code review effort: 3 (Moderate) | ~30 minutes

Possibly related PRs

Suggested labels: type: bug

Suggested reviewers: OtavioStasiak

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: pixel-snapping shared row heights to make separators render consistently.
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-1344: 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.

@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.

Actionable comments posted: 3

🧹 Nitpick comments (2)
app/containers/ActionSheet/useActionSheetItemHeight.ts (1)

5-8: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Add an explicit return type to the hook.

As per coding guidelines: "add explicit type annotations to function parameters and return types."

♻️ Proposed change
-export const useActionSheetItemHeight = () => {
+export const useActionSheetItemHeight = (): number => {
 	const { fontScale } = useWindowDimensions();
 	return PixelRatio.roundToNearestPixel(ACTION_SHEET_ITEM_HEIGHT * fontScale);
 };
🤖 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/ActionSheet/useActionSheetItemHeight.ts` around lines 5 - 8,
The useActionSheetItemHeight hook is missing an explicit return type annotation.
Update useActionSheetItemHeight to declare its return type directly on the
function signature, keeping the existing fontScale and
PixelRatio.roundToNearestPixel logic unchanged, so it conforms to the project’s
type annotation guidelines.

Source: Coding guidelines

app/containers/ActionSheet/useActionSheetDetents.ts (1)

51-51: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

CANCEL_HEIGHT duplicates the already-supplied itemHeight.

itemHeight is passed in from ActionSheet.tsx where it is also useActionSheetItemHeight(), so CANCEL_HEIGHT is always identical to itemHeight. The extra hook call and its dependency-array entry are redundant; reusing itemHeight would simplify the memo and avoid the two ever drifting apart.

🤖 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/ActionSheet/useActionSheetDetents.ts` at line 51, The
CANCEL_HEIGHT value in useActionSheetDetents is duplicating the already
available itemHeight from ActionSheet.tsx, so remove the extra
useActionSheetItemHeight() call and use itemHeight directly in the detent memo.
Update the useMemo dependency list accordingly so it depends on itemHeight
rather than a separate CANCEL_HEIGHT source, keeping the detents logic in sync.
🤖 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/ActionSheet/useActionSheetItemHeight.ts`:
- Around line 5-8: The `useActionSheetItemHeight` hook is reading `fontScale`
directly from `useWindowDimensions`, which diverges from the shared responsive
layout source used by `DirectoryItem`, `UserItem`, and `DepartmentItemFilter`.
Update `useActionSheetItemHeight` to use `useResponsiveLayout` instead, and
derive the item height from its `fontScale` value so all row sizing follows the
same responsive abstraction and stays consistent if the layout provider changes.

In `@app/views/SelectServerView.tsx`:
- Around line 29-35: The `getItemLayout` callback in `SelectServerView` is
calculating offsets using only the row height, but the `FlatList` also renders
`List.Separator` for the header, footer, and between items. Update
`getItemLayout` to include the separator’s vertical height in both the returned
`length`/`offset` math so it stays aligned with the `FlatList` separators and
does not drift during scrolling; use the existing `List.Separator` behavior and
the `getItemLayout` symbol to keep the calculation consistent with `ROW_HEIGHT`
and `fontScale`.

In `@app/views/ShareListView/index.tsx`:
- Around line 66-69: `getItemLayout` in `ShareListView/index.tsx` has two layout
contract mismatches that can cause `FlatList` offsets to drift: it should use
the same font-scale source as `DirectoryItem` (the
`useResponsiveLayout().fontScale` / `useWindowDimensions`-based value) instead
of `PixelRatio.getFontScale()`, and its `offset` calculation must account for
`List.Separator` by including the separator height between rows. Update the
`getItemLayout` helper so the computed length/offset matches the actual rendered
row height plus separator spacing.

---

Nitpick comments:
In `@app/containers/ActionSheet/useActionSheetDetents.ts`:
- Line 51: The CANCEL_HEIGHT value in useActionSheetDetents is duplicating the
already available itemHeight from ActionSheet.tsx, so remove the extra
useActionSheetItemHeight() call and use itemHeight directly in the detent memo.
Update the useMemo dependency list accordingly so it depends on itemHeight
rather than a separate CANCEL_HEIGHT source, keeping the detents logic in sync.

In `@app/containers/ActionSheet/useActionSheetItemHeight.ts`:
- Around line 5-8: The useActionSheetItemHeight hook is missing an explicit
return type annotation. Update useActionSheetItemHeight to declare its return
type directly on the function signature, keeping the existing fontScale and
PixelRatio.roundToNearestPixel logic unchanged, so it conforms to the project’s
type annotation guidelines.
🪄 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: 3586431a-6839-4d58-8806-e59c3ccbe5f6

📥 Commits

Reviewing files that changed from the base of the PR and between a8c4899 and f083c42.

⛔ Files ignored due to path filters (2)
  • app/containers/ServerItem/__snapshots__/ServerItem.test.tsx.snap is excluded by !**/*.snap
  • app/views/NewServerView/components/ServersHistoryItem/__snapshots__/ServersHistoryItem.test.tsx.snap is excluded by !**/*.snap
📒 Files selected for processing (18)
  • app/containers/ActionSheet/ActionSheet.tsx
  • app/containers/ActionSheet/BottomSheetContent.tsx
  • app/containers/ActionSheet/Item.tsx
  • app/containers/ActionSheet/useActionSheetDetents.ts
  • app/containers/ActionSheet/useActionSheetItemHeight.ts
  • app/containers/DirectoryItem/index.tsx
  • app/containers/List/ListItem.tsx
  • app/containers/ServerItem/index.tsx
  • app/containers/ServerItem/styles.ts
  • app/containers/UserItem.tsx
  • app/ee/omnichannel/views/QueueListView.tsx
  • app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx
  • app/views/CannedResponsesListView/DepartmentFilter/DepartmentItemFilter.tsx
  • app/views/NewServerView/components/ServersHistoryItem/index.tsx
  • app/views/NewServerView/components/ServersHistoryItem/styles.ts
  • app/views/SelectServerView.tsx
  • app/views/ShareListView/index.tsx
  • app/views/TeamChannelsView.tsx
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
  • GitHub Check: ESLint and Test / run-eslint-and-test
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{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/views/NewServerView/components/ServersHistoryItem/styles.ts
  • app/containers/ServerItem/styles.ts
  • app/containers/DirectoryItem/index.tsx
  • app/containers/ActionSheet/useActionSheetItemHeight.ts
  • app/containers/ActionSheet/ActionSheet.tsx
  • app/containers/ActionSheet/BottomSheetContent.tsx
  • app/containers/ServerItem/index.tsx
  • app/containers/UserItem.tsx
  • app/ee/omnichannel/views/QueueListView.tsx
  • app/views/CannedResponsesListView/DepartmentFilter/DepartmentItemFilter.tsx
  • app/views/ShareListView/index.tsx
  • app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx
  • app/views/NewServerView/components/ServersHistoryItem/index.tsx
  • app/containers/List/ListItem.tsx
  • app/views/SelectServerView.tsx
  • app/containers/ActionSheet/useActionSheetDetents.ts
  • app/containers/ActionSheet/Item.tsx
  • app/views/TeamChannelsView.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/views/NewServerView/components/ServersHistoryItem/styles.ts
  • app/containers/ServerItem/styles.ts
  • app/containers/DirectoryItem/index.tsx
  • app/containers/ActionSheet/useActionSheetItemHeight.ts
  • app/containers/ActionSheet/ActionSheet.tsx
  • app/containers/ActionSheet/BottomSheetContent.tsx
  • app/containers/ServerItem/index.tsx
  • app/containers/UserItem.tsx
  • app/ee/omnichannel/views/QueueListView.tsx
  • app/views/CannedResponsesListView/DepartmentFilter/DepartmentItemFilter.tsx
  • app/views/ShareListView/index.tsx
  • app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx
  • app/views/NewServerView/components/ServersHistoryItem/index.tsx
  • app/containers/List/ListItem.tsx
  • app/views/SelectServerView.tsx
  • app/containers/ActionSheet/useActionSheetDetents.ts
  • app/containers/ActionSheet/Item.tsx
  • app/views/TeamChannelsView.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/views/NewServerView/components/ServersHistoryItem/styles.ts
  • app/containers/ServerItem/styles.ts
  • app/containers/DirectoryItem/index.tsx
  • app/containers/ActionSheet/useActionSheetItemHeight.ts
  • app/containers/ActionSheet/ActionSheet.tsx
  • app/containers/ActionSheet/BottomSheetContent.tsx
  • app/containers/ServerItem/index.tsx
  • app/containers/UserItem.tsx
  • app/ee/omnichannel/views/QueueListView.tsx
  • app/views/CannedResponsesListView/DepartmentFilter/DepartmentItemFilter.tsx
  • app/views/ShareListView/index.tsx
  • app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx
  • app/views/NewServerView/components/ServersHistoryItem/index.tsx
  • app/containers/List/ListItem.tsx
  • app/views/SelectServerView.tsx
  • app/containers/ActionSheet/useActionSheetDetents.ts
  • app/containers/ActionSheet/Item.tsx
  • app/views/TeamChannelsView.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/views/NewServerView/components/ServersHistoryItem/styles.ts
  • app/containers/ServerItem/styles.ts
  • app/containers/DirectoryItem/index.tsx
  • app/containers/ActionSheet/useActionSheetItemHeight.ts
  • app/containers/ActionSheet/ActionSheet.tsx
  • app/containers/ActionSheet/BottomSheetContent.tsx
  • app/containers/ServerItem/index.tsx
  • app/containers/UserItem.tsx
  • app/ee/omnichannel/views/QueueListView.tsx
  • app/views/CannedResponsesListView/DepartmentFilter/DepartmentItemFilter.tsx
  • app/views/ShareListView/index.tsx
  • app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx
  • app/views/NewServerView/components/ServersHistoryItem/index.tsx
  • app/containers/List/ListItem.tsx
  • app/views/SelectServerView.tsx
  • app/containers/ActionSheet/useActionSheetDetents.ts
  • app/containers/ActionSheet/Item.tsx
  • app/views/TeamChannelsView.tsx
app/views/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Place screen components in 'app/views/' directory

Files:

  • app/views/NewServerView/components/ServersHistoryItem/styles.ts
  • app/views/CannedResponsesListView/DepartmentFilter/DepartmentItemFilter.tsx
  • app/views/ShareListView/index.tsx
  • app/views/NewServerView/components/ServersHistoryItem/index.tsx
  • app/views/SelectServerView.tsx
  • app/views/TeamChannelsView.tsx
app/containers/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

  • app/containers/ServerItem/styles.ts
  • app/containers/DirectoryItem/index.tsx
  • app/containers/ActionSheet/useActionSheetItemHeight.ts
  • app/containers/ActionSheet/ActionSheet.tsx
  • app/containers/ActionSheet/BottomSheetContent.tsx
  • app/containers/ServerItem/index.tsx
  • app/containers/UserItem.tsx
  • app/containers/List/ListItem.tsx
  • app/containers/ActionSheet/useActionSheetDetents.ts
  • app/containers/ActionSheet/Item.tsx
app/ee/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Place enterprise features (Omnichannel/livechat) in 'app/ee/' directory

Files:

  • app/ee/omnichannel/views/QueueListView.tsx
app/lib/hooks/useResponsiveLayout/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Use responsive layout hooks from 'app/lib/hooks/useResponsiveLayout/' for master-detail on tablets and single stack on phones

Files:

  • app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx
🧠 Learnings (3)
📚 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/views/NewServerView/components/ServersHistoryItem/styles.ts
  • app/containers/ServerItem/styles.ts
  • app/containers/DirectoryItem/index.tsx
  • app/containers/ActionSheet/useActionSheetItemHeight.ts
  • app/containers/ActionSheet/ActionSheet.tsx
  • app/containers/ActionSheet/BottomSheetContent.tsx
  • app/containers/ServerItem/index.tsx
  • app/containers/UserItem.tsx
  • app/ee/omnichannel/views/QueueListView.tsx
  • app/views/CannedResponsesListView/DepartmentFilter/DepartmentItemFilter.tsx
  • app/views/ShareListView/index.tsx
  • app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx
  • app/views/NewServerView/components/ServersHistoryItem/index.tsx
  • app/containers/List/ListItem.tsx
  • app/views/SelectServerView.tsx
  • app/containers/ActionSheet/useActionSheetDetents.ts
  • app/containers/ActionSheet/Item.tsx
  • app/views/TeamChannelsView.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/DirectoryItem/index.tsx
  • app/containers/ActionSheet/ActionSheet.tsx
  • app/containers/ActionSheet/BottomSheetContent.tsx
  • app/containers/ServerItem/index.tsx
  • app/containers/UserItem.tsx
  • app/ee/omnichannel/views/QueueListView.tsx
  • app/views/CannedResponsesListView/DepartmentFilter/DepartmentItemFilter.tsx
  • app/views/ShareListView/index.tsx
  • app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx
  • app/views/NewServerView/components/ServersHistoryItem/index.tsx
  • app/containers/List/ListItem.tsx
  • app/views/SelectServerView.tsx
  • app/containers/ActionSheet/Item.tsx
  • app/views/TeamChannelsView.tsx
📚 Learning: 2026-06-24T22:58:43.390Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7157
File: app/views/MessagesView/index.tsx:392-392
Timestamp: 2026-06-24T22:58:43.390Z
Learning: When wrapping a React Native component (e.g., via `withSafeAreaInsets`) ensure `hoistNonReactStatics` is only required if the wrapped component actually defines static properties/methods that consumers rely on. If the component has no statics (as in `app/views/MessagesView/index.tsx`), you can omit `hoistNonReactStatics` for this case.

Applied to files:

  • app/views/CannedResponsesListView/DepartmentFilter/DepartmentItemFilter.tsx
  • app/views/ShareListView/index.tsx
  • app/views/NewServerView/components/ServersHistoryItem/index.tsx
  • app/views/SelectServerView.tsx
  • app/views/TeamChannelsView.tsx
🔇 Additional comments (10)
app/containers/ActionSheet/ActionSheet.tsx (1)

30-42: LGTM!

app/containers/ActionSheet/BottomSheetContent.tsx (1)

43-51: LGTM!

app/containers/ActionSheet/Item.tsx (1)

24-50: LGTM!

app/containers/ActionSheet/useActionSheetDetents.ts (1)

53-93: LGTM!

app/containers/DirectoryItem/index.tsx (1)

50-52: LGTM!

app/views/NewServerView/components/ServersHistoryItem/styles.ts (1)

5-11: LGTM!

app/ee/omnichannel/views/QueueListView.tsx (1)

71-78: LGTM!

app/views/TeamChannelsView.tsx (2)

568-568: LGTM!


511-520: 🩺 Stability & Availability

PixelRatio is correctly imported; no runtime error.

The import import { ... PixelRatio } from 'react-native'; exists at line 3, so getItemLayout will not throw a ReferenceError. The original concern regarding a missing import is incorrect.

			> Likely an incorrect or invalid review comment.
app/views/NewServerView/components/ServersHistoryItem/index.tsx (1)

24-38: 🎯 Functional Correctness

Remove unfounded getItemLayout concern

The risk of scroll offset drift due to a hardcoded getItemLayout does not exist for this component. Verification confirms ServersHistoryItem is rendered as a static list of components within ServersHistoryActionSheetContent (wrapped in Fragment and A11y helpers), not via a virtualized list (e.g., FlatList) that would utilize getItemLayout.

No getItemLayout implementation consuming these items was found in NewServerView. The ROW_HEIGHT constant is correctly used for the inline height calculation in the component itself.

			> Likely an incorrect or invalid review comment.

Comment thread app/containers/ActionSheet/useActionSheetItemHeight.ts Outdated
Comment thread app/views/SelectServerView.tsx
Comment thread app/views/ShareListView/index.tsx
@github-actions

Copy link
Copy Markdown

iOS Build Available

Rocket.Chat 4.74.0.109227

@github-actions

Copy link
Copy Markdown

@github-actions

Copy link
Copy Markdown

iOS Build Available

Rocket.Chat 4.74.0.109229

@github-actions

Copy link
Copy Markdown

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown

iOS Build Available

Rocket.Chat 4.74.0.109249

@OtavioStasiak OtavioStasiak 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.

LGTM!
Just add a before and after screenshot on the PR description :)

…he pixel grid

Some lists intermittently failed to render the 1px List.Separator, and some
rendered it with non-uniform thickness (RoomsListView, MessageActions action
sheet, and others).

Root cause: rows whose rendered height in DP did not land on the device
physical-pixel grid caused the hairline separator to anti-alias below
visibility on some rows and across multiple physical pixels on others. As
cumulative row offsets drifted fractionally, each successive separator landed
at a different sub-pixel phase.

Fix: snap each row container height to the pixel grid with
PixelRatio.roundToNearestPixel(height * fontScale) so every separator sits at
an integer physical-pixel offset and renders as a crisp uniform hairline. The
snap is a no-op where the height was already integer-px and corrective
otherwise. Applied to every shared list-row component (RoomItem via the
useResponsiveLayout rowHeight SSOT, ActionSheet Item via a new
useActionSheetItemHeight hook, List.Item, DirectoryItem, UserItem, ServerItem,
ServersHistoryItem, DepartmentItemFilter).

Also fixes a secondary bug class: incorrect getItemLayout scroll-math in
SelectServerView, QueueListView, TeamChannelsView and ShareListView (two used
the array length as the per-row pixel height; offsets ignored pixel-snapping
and condensed display mode).

Ref: NATIVE-1344

Claude-Session: https://claude.ai/code/session_01CrrW51BkNx2qDu1PZWgK5y
Address review feedback:
- SelectServerView and ShareListView getItemLayout now add the row
  separator height to each offset, so the virtualization math matches the
  actual rendered positions of separated rows.
- useActionSheetDetents reuses the itemHeight it already receives instead
  of calling useActionSheetItemHeight again, which always returned the
  same value.
- Add an explicit return type to useActionSheetItemHeight.

Claude-Session: https://claude.ai/code/session_01PbWnUn8q3EUbGkNWyX33c9
The reactions header rendered without an explicit height, so its height
settled from the async layout of the inner horizontal emoji list.
TrueSheet's floating native header sized to that settling content, and on
the first present the header overlay briefly covered the first inter-row
separator (Edit/Quote) until a relayout repainted it. This reproduced only
on real iOS devices at @3x, not Android or the simulator, which is why the
earlier pixel-grid row-height fix could not address it.

Pin the container to HEADER_HEIGHT so the header height is stable from the
first frame and never depends on the async emoji-list layout.
@diegolmello
diegolmello had a problem deploying to approve_e2e_testing July 1, 2026 17:48 — with GitHub Actions Failure
@diegolmello
diegolmello merged commit b4e58d7 into develop Jul 1, 2026
4 of 7 checks passed
@diegolmello
diegolmello deleted the stormy-count branch July 1, 2026 17:50
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