feat: migrate MessageSearch to PaginatedVirtualList#40844
Open
srijnabhargav wants to merge 3 commits into
Open
feat: migrate MessageSearch to PaginatedVirtualList#40844srijnabhargav wants to merge 3 commits into
srijnabhargav wants to merge 3 commits into
Conversation
Contributor
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (2)
📜 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). (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.{ts,tsx,js}📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
Files:
🧠 Learnings (2)📚 Learning: 2026-03-27T14:52:56.865ZApplied to files:
📚 Learning: 2026-05-06T12:21:44.083ZApplied to files:
🔇 Additional comments (2)
WalkthroughThis PR extracts message-search rendering into a new MessageSearch component, converts useMessageSearchQuery to useInfiniteQuery (infinite pagination), and simplifies MessageSearchTab to pass query results and UI context into MessageSearch. ChangesMessage Search Pagination and Component Extraction
Sequence DiagramsequenceDiagram
participant MessageSearchTab
participant MessageSearch
participant useMessageSearchQuery
participant PaginatedVirtualList
MessageSearchTab->>MessageSearch: pass query props & UI context
MessageSearch->>useMessageSearchQuery: receive paginated data/status
useMessageSearchQuery-->>MessageSearch: items, itemCount, isPending, isSuccess, fetchNextPage
MessageSearch->>PaginatedVirtualList: render items, onEndReached -> fetchNextPage
PaginatedVirtualList->>MessageSearch: trigger fetchNextPage
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ 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. Comment |
coderabbitai
Bot
added
the
type: feature
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
apps/meteor/client/components/VirtualList/VirtuaListContainer.tsx (1)
10-13: ⚡ Quick winMake
styleoptional in the container props API.
styleis currently required inVirtuaListContainerProps, but this component safely handles missing style and HTMLstyleis normally optional. Requiring it makes reuse unnecessarily strict.export type VirtuaListContainerProps = { children: ReactNode; - style: CSSProperties; + style?: CSSProperties; } & Omit<HTMLAttributes<HTMLUListElement>, 'children' | 'style'>;🤖 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 `@apps/meteor/client/components/VirtualList/VirtuaListContainer.tsx` around lines 10 - 13, The props type VirtuaListContainerProps currently marks style as required; update it to make style optional by changing the declaration from "style: CSSProperties" to "style?: CSSProperties" in the VirtuaListContainerProps type (keeping children: ReactNode and the Omit of HTMLAttributes<HTMLUListElement> intact) so the component API matches how the component handles missing style and aligns with standard HTML optional style props.apps/meteor/client/views/room/contextualBar/MessageSearchTab/MessageSearchTab.tsx (1)
30-31: 💤 Low valueConsider lifting query data from
MessageSearchto avoid duplicate subscriptions.Both
MessageSearchTabandMessageSearchcalluseMessageSearchQuerywith identical parameters. While React Query deduplicates network requests, both components maintain separate subscriptions. Passingdataand query state as props toMessageSearchwould reduce overhead, though the current approach is functionally correct.🤖 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 `@apps/meteor/client/views/room/contextualBar/MessageSearchTab/MessageSearchTab.tsx` around lines 30 - 31, MessageSearchTab and MessageSearch both call useMessageSearchQuery with the same args, causing duplicate React Query subscriptions; refactor by lifting the query state out of MessageSearch so MessageSearchTab calls useMessageSearchQuery once (keep const { isSuccess, data, isPending } = useMessageSearchQuery({ searchText, globalSearch }) and const itemCount = data?.itemCount ?? 0) and pass the resulting data and state props (data, isSuccess, isPending, itemCount) into the MessageSearch component instead of letting MessageSearch call useMessageSearchQuery itself; update MessageSearch’s props/signature accordingly and remove its internal call to useMessageSearchQuery.apps/meteor/client/views/room/contextualBar/Discussions/DiscussionsList.tsx (1)
82-82: 💤 Low valueConsider memoizing
renderItemcallback to prevent unnecessary re-renders.The inline
renderItemfunction creates a new reference on every render. IfPaginatedVirtualListuses reference equality checks for optimization, this could cause all visible items to re-render even whendiscussions,showRealNames, andonClickhaven't changed.♻️ Suggested refactor
+ const renderDiscussionRow = useCallback( + (discussion: IDiscussionMessage) => ( + <DiscussionsListRow discussion={discussion} showRealNames={showRealNames} onClick={onClick} /> + ), + [showRealNames, onClick], + );Then use
renderItem={renderDiscussionRow}in the JSX.Also applies to: 98-108
🤖 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 `@apps/meteor/client/views/room/contextualBar/Discussions/DiscussionsList.tsx` at line 82, The inline renderItem passed to PaginatedVirtualList is recreated each render causing unnecessary re-renders; extract it into a memoized callback (e.g., create a function named renderDiscussionRow and wrap it with React.useCallback) and replace the inline renderItem with renderDiscussionRow; ensure the useCallback dependency array includes only discussions-relevant values like showRealNames and onClick (and any props used inside) so the function reference remains stable when those values don't change; apply the same change for the other renderItem occurrences in this file that render discussion rows.
🤖 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 `@apps/meteor/client/components/VirtualList/VirtuaListContainer.tsx`:
- Around line 10-13: The props type VirtuaListContainerProps currently marks
style as required; update it to make style optional by changing the declaration
from "style: CSSProperties" to "style?: CSSProperties" in the
VirtuaListContainerProps type (keeping children: ReactNode and the Omit of
HTMLAttributes<HTMLUListElement> intact) so the component API matches how the
component handles missing style and aligns with standard HTML optional style
props.
In `@apps/meteor/client/views/room/contextualBar/Discussions/DiscussionsList.tsx`:
- Line 82: The inline renderItem passed to PaginatedVirtualList is recreated
each render causing unnecessary re-renders; extract it into a memoized callback
(e.g., create a function named renderDiscussionRow and wrap it with
React.useCallback) and replace the inline renderItem with renderDiscussionRow;
ensure the useCallback dependency array includes only discussions-relevant
values like showRealNames and onClick (and any props used inside) so the
function reference remains stable when those values don't change; apply the same
change for the other renderItem occurrences in this file that render discussion
rows.
In
`@apps/meteor/client/views/room/contextualBar/MessageSearchTab/MessageSearchTab.tsx`:
- Around line 30-31: MessageSearchTab and MessageSearch both call
useMessageSearchQuery with the same args, causing duplicate React Query
subscriptions; refactor by lifting the query state out of MessageSearch so
MessageSearchTab calls useMessageSearchQuery once (keep const { isSuccess, data,
isPending } = useMessageSearchQuery({ searchText, globalSearch }) and const
itemCount = data?.itemCount ?? 0) and pass the resulting data and state props
(data, isSuccess, isPending, itemCount) into the MessageSearch component instead
of letting MessageSearch call useMessageSearchQuery itself; update
MessageSearch’s props/signature accordingly and remove its internal call to
useMessageSearchQuery.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 83ab1447-f76b-4b87-b211-ca250bd28622
⛔ Files ignored due to path filters (1)
apps/meteor/client/views/room/contextualBar/Discussions/__snapshots__/DiscussionsList.spec.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (15)
apps/meteor/client/components/VirtualList/VirtuaListContainer.tsxapps/meteor/client/components/VirtualList/VirtualList.spec.tsxapps/meteor/client/components/VirtualList/VirtualList.tsxapps/meteor/client/components/VirtualList/index.tsapps/meteor/client/views/room/MessageList/MessageList.spec.tsxapps/meteor/client/views/room/contextualBar/Discussions/DiscussionsList.spec.tsxapps/meteor/client/views/room/contextualBar/Discussions/DiscussionsList.stories.tsxapps/meteor/client/views/room/contextualBar/Discussions/DiscussionsList.tsxapps/meteor/client/views/room/contextualBar/Discussions/DiscussionsListContextBar.tsxapps/meteor/client/views/room/contextualBar/Discussions/components/DiscussionsListItem.stories.tsxapps/meteor/client/views/room/contextualBar/Discussions/components/DiscussionsListItem.tsxapps/meteor/client/views/room/contextualBar/MessageSearchTab/MessageSearchTab.tsxapps/meteor/client/views/room/contextualBar/MessageSearchTab/components/MessageSearch.spec.tsxapps/meteor/client/views/room/contextualBar/MessageSearchTab/components/MessageSearch.tsxapps/meteor/client/views/room/contextualBar/MessageSearchTab/hooks/useMessageSearchQuery.ts
📜 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: cubic · AI code reviewer
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/client/components/VirtualList/VirtuaListContainer.tsxapps/meteor/client/views/room/contextualBar/Discussions/components/DiscussionsListItem.stories.tsxapps/meteor/client/views/room/contextualBar/Discussions/DiscussionsListContextBar.tsxapps/meteor/client/components/VirtualList/index.tsapps/meteor/client/views/room/MessageList/MessageList.spec.tsxapps/meteor/client/views/room/contextualBar/Discussions/DiscussionsList.spec.tsxapps/meteor/client/components/VirtualList/VirtualList.tsxapps/meteor/client/views/room/contextualBar/MessageSearchTab/components/MessageSearch.spec.tsxapps/meteor/client/views/room/contextualBar/Discussions/components/DiscussionsListItem.tsxapps/meteor/client/views/room/contextualBar/Discussions/DiscussionsList.stories.tsxapps/meteor/client/views/room/contextualBar/MessageSearchTab/components/MessageSearch.tsxapps/meteor/client/views/room/contextualBar/Discussions/DiscussionsList.tsxapps/meteor/client/views/room/contextualBar/MessageSearchTab/hooks/useMessageSearchQuery.tsapps/meteor/client/components/VirtualList/VirtualList.spec.tsxapps/meteor/client/views/room/contextualBar/MessageSearchTab/MessageSearchTab.tsx
🧠 Learnings (31)
📓 Common learnings
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 40755
File: apps/meteor/client/views/room/MessageList/MessageList.tsx:84-94
Timestamp: 2026-06-02T13:27:22.143Z
Learning: In `apps/meteor/client/views/room/MessageList/MessageList.tsx`, the keep-at-bottom `useEffect` intentionally calls `virtualizerRef.current.scrollToIndex(messagesLength, { align: 'end' })` where `messagesLength` is one past the last rendered item index. Using `messagesLength - 1` was tested and caused incorrect scroll positioning. The out-of-bounds index is clamped by Virtua to `itemCount - 1` (last item) — this is intentional and relies on documented stable behavior of the Virtua library. Do not flag this as a bug.
📚 Learning: 2026-06-02T13:27:22.143Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 40755
File: apps/meteor/client/views/room/MessageList/MessageList.tsx:84-94
Timestamp: 2026-06-02T13:27:22.143Z
Learning: In `apps/meteor/client/views/room/MessageList/MessageList.tsx`, the keep-at-bottom `useEffect` intentionally calls `virtualizerRef.current.scrollToIndex(messagesLength, { align: 'end' })` where `messagesLength` is one past the last rendered item index. Using `messagesLength - 1` was tested and caused incorrect scroll positioning. The out-of-bounds index is clamped by Virtua to `itemCount - 1` (last item) — this is intentional and relies on documented stable behavior of the Virtua library. Do not flag this as a bug.
Applied to files:
apps/meteor/client/components/VirtualList/VirtuaListContainer.tsxapps/meteor/client/views/room/contextualBar/Discussions/DiscussionsListContextBar.tsxapps/meteor/client/components/VirtualList/index.tsapps/meteor/client/views/room/MessageList/MessageList.spec.tsxapps/meteor/client/views/room/contextualBar/Discussions/DiscussionsList.spec.tsxapps/meteor/client/components/VirtualList/VirtualList.tsxapps/meteor/client/views/room/contextualBar/MessageSearchTab/components/MessageSearch.tsxapps/meteor/client/views/room/contextualBar/Discussions/DiscussionsList.tsxapps/meteor/client/views/room/contextualBar/MessageSearchTab/hooks/useMessageSearchQuery.tsapps/meteor/client/components/VirtualList/VirtualList.spec.tsxapps/meteor/client/views/room/contextualBar/MessageSearchTab/MessageSearchTab.tsx
📚 Learning: 2026-03-27T14:52:56.865Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39892
File: apps/meteor/client/views/room/contextualBar/Threads/Thread.tsx:150-155
Timestamp: 2026-03-27T14:52:56.865Z
Learning: In Rocket.Chat, there are two different `ModalBackdrop` components with different prop APIs. During review, confirm the import source: (1) `rocket.chat/fuselage` `ModalBackdrop` uses `ModalBackdropProps` based on `BoxProps` (so it supports `onClick` and other Box/DOM props) and does not have an `onDismiss` prop; (2) `rocket.chat/ui-client` `ModalBackdrop` uses a narrower props interface like `{ children?: ReactNode; onDismiss?: () => void }` and handles Escape keypress and outside mouse-up, and it does not forward arbitrary DOM props such as `onClick`. Flag mismatched props (e.g., `onDismiss` passed to the fuselage component or `onClick` passed to the ui-client component) and ensure the usage matches the correct component being imported.
Applied to files:
apps/meteor/client/components/VirtualList/VirtuaListContainer.tsxapps/meteor/client/views/room/contextualBar/Discussions/components/DiscussionsListItem.stories.tsxapps/meteor/client/views/room/contextualBar/Discussions/DiscussionsListContextBar.tsxapps/meteor/client/views/room/MessageList/MessageList.spec.tsxapps/meteor/client/views/room/contextualBar/Discussions/DiscussionsList.spec.tsxapps/meteor/client/components/VirtualList/VirtualList.tsxapps/meteor/client/views/room/contextualBar/MessageSearchTab/components/MessageSearch.spec.tsxapps/meteor/client/views/room/contextualBar/Discussions/components/DiscussionsListItem.tsxapps/meteor/client/views/room/contextualBar/Discussions/DiscussionsList.stories.tsxapps/meteor/client/views/room/contextualBar/MessageSearchTab/components/MessageSearch.tsxapps/meteor/client/views/room/contextualBar/Discussions/DiscussionsList.tsxapps/meteor/client/components/VirtualList/VirtualList.spec.tsxapps/meteor/client/views/room/contextualBar/MessageSearchTab/MessageSearchTab.tsx
📚 Learning: 2026-05-06T12:21:44.083Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 40256
File: apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx:121-149
Timestamp: 2026-05-06T12:21:44.083Z
Learning: Field wrappers in rocket.chat/fuselage-forms (Field, FieldLabel, FieldRow, FieldError, FieldHint) auto-create htmlFor/id associations, aria-describedby, and role="alert" for errors. Do not manually set htmlFor, id, aria-describedby, or role attributes when using these wrappers. This automatic wiring does not apply to plain rocket.chat/fuselage components, which require explicit ID wiring per the accessibility docs. In code reviews, prefer using fuselage-forms wrappers for form fields and verify there is no unnecessary manual ID/aria wiring in files that use these wrappers. If a component uses plain fuselage components, ensure proper id wiring as per docs.
Applied to files:
apps/meteor/client/components/VirtualList/VirtuaListContainer.tsxapps/meteor/client/views/room/contextualBar/Discussions/components/DiscussionsListItem.stories.tsxapps/meteor/client/views/room/contextualBar/Discussions/DiscussionsListContextBar.tsxapps/meteor/client/components/VirtualList/index.tsapps/meteor/client/views/room/MessageList/MessageList.spec.tsxapps/meteor/client/views/room/contextualBar/Discussions/DiscussionsList.spec.tsxapps/meteor/client/components/VirtualList/VirtualList.tsxapps/meteor/client/views/room/contextualBar/MessageSearchTab/components/MessageSearch.spec.tsxapps/meteor/client/views/room/contextualBar/Discussions/components/DiscussionsListItem.tsxapps/meteor/client/views/room/contextualBar/Discussions/DiscussionsList.stories.tsxapps/meteor/client/views/room/contextualBar/MessageSearchTab/components/MessageSearch.tsxapps/meteor/client/views/room/contextualBar/Discussions/DiscussionsList.tsxapps/meteor/client/views/room/contextualBar/MessageSearchTab/hooks/useMessageSearchQuery.tsapps/meteor/client/components/VirtualList/VirtualList.spec.tsxapps/meteor/client/views/room/contextualBar/MessageSearchTab/MessageSearchTab.tsx
📚 Learning: 2026-02-24T19:36:55.089Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/page-objects/fragments/home-content.ts:60-82
Timestamp: 2026-02-24T19:36:55.089Z
Learning: In RocketChat/Rocket.Chat e2e tests (apps/meteor/tests/e2e/page-objects/fragments/home-content.ts), thread message preview listitems do not have aria-roledescription="message", so lastThreadMessagePreview locator cannot be scoped to messageListItems (which filters for aria-roledescription="message"). It should remain scoped to page.getByRole('listitem') or mainMessageList.getByRole('listitem').
Applied to files:
apps/meteor/client/views/room/contextualBar/Discussions/components/DiscussionsListItem.stories.tsxapps/meteor/client/views/room/MessageList/MessageList.spec.tsxapps/meteor/client/views/room/contextualBar/Discussions/DiscussionsList.spec.tsxapps/meteor/client/views/room/contextualBar/MessageSearchTab/components/MessageSearch.spec.tsxapps/meteor/client/views/room/contextualBar/Discussions/DiscussionsList.stories.tsxapps/meteor/client/views/room/contextualBar/Discussions/DiscussionsList.tsxapps/meteor/client/views/room/contextualBar/MessageSearchTab/hooks/useMessageSearchQuery.tsapps/meteor/client/components/VirtualList/VirtualList.spec.tsxapps/meteor/client/views/room/contextualBar/MessageSearchTab/MessageSearchTab.tsx
📚 Learning: 2026-01-17T01:51:47.764Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38219
File: packages/core-typings/src/cloud/Announcement.ts:5-6
Timestamp: 2026-01-17T01:51:47.764Z
Learning: In packages/core-typings/src/cloud/Announcement.ts, the AnnouncementSchema.createdBy field intentionally overrides IBannerSchema.createdBy (object with _id and optional username) with a string enum ['cloud', 'system'] to match existing runtime behavior. This is documented as technical debt with a FIXME comment at apps/meteor/app/cloud/server/functions/syncWorkspace/handleCommsSync.ts:53 and should not be flagged as an error until the runtime behavior is corrected.
Applied to files:
apps/meteor/client/views/room/contextualBar/Discussions/components/DiscussionsListItem.stories.tsx
📚 Learning: 2026-02-10T16:32:42.586Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38528
File: apps/meteor/client/startup/roles.ts:14-14
Timestamp: 2026-02-10T16:32:42.586Z
Learning: In Rocket.Chat's Meteor client code, DDP streams use EJSON and Date fields arrive as Date objects; do not manually construct new Date() in stream handlers (for example, in sdk.stream()). Only REST API responses return plain JSON where dates are strings, so implement explicit conversion there if needed. Apply this guidance to all TypeScript files under apps/meteor/client to ensure consistent date handling in DDP streams and REST responses.
Applied to files:
apps/meteor/client/components/VirtualList/index.tsapps/meteor/client/views/room/contextualBar/MessageSearchTab/hooks/useMessageSearchQuery.ts
📚 Learning: 2026-05-11T20:30:35.265Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 40480
File: apps/meteor/client/meteor/startup/accounts.ts:59-61
Timestamp: 2026-05-11T20:30:35.265Z
Learning: In Rocket.Chat’s Meteor client code, when calling `dispatchToastMessage` with `{ type: 'error' }`, pass the raw caught error object as `message` without manual normalization. `dispatchToastMessage` is designed to accept `message: unknown` for error toasts, so avoid converting errors to strings (e.g., `String(error)`) or extracting `error.message` before passing them.
Applied to files:
apps/meteor/client/components/VirtualList/index.tsapps/meteor/client/views/room/contextualBar/MessageSearchTab/hooks/useMessageSearchQuery.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In the Rocket.Chat repository, do not reference Biome lint rules in code review feedback. Biome is not used even if biome.json exists; only reference Biome rules if there is explicit, project-wide usage documented. For TypeScript files, review lint implications without Biome guidance unless the project enables Biome rules.
Applied to files:
apps/meteor/client/components/VirtualList/index.tsapps/meteor/client/views/room/contextualBar/MessageSearchTab/hooks/useMessageSearchQuery.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.
Applied to files:
apps/meteor/client/components/VirtualList/index.tsapps/meteor/client/views/room/contextualBar/MessageSearchTab/hooks/useMessageSearchQuery.ts
📚 Learning: 2026-04-17T18:33:27.211Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 39858
File: apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts:123-151
Timestamp: 2026-04-17T18:33:27.211Z
Learning: In RocketChat/Rocket.Chat (`apps/meteor/tests/e2e/apps/uikit-interactions.spec.ts`), `executeBlockActionHandler` invocations originating from a **modal** surface intentionally do NOT include a `block_action_room` (room property) in the interaction payload. Modals are not scoped to a room, so no room id is available in that context. Do not flag the absence of a room assertion in the modal block-action test as a missing coverage bug; instead, document it explicitly with a `test.step` asserting the room entry is `undefined`.
Applied to files:
apps/meteor/client/views/room/MessageList/MessageList.spec.tsxapps/meteor/client/views/room/contextualBar/Discussions/DiscussionsList.spec.tsxapps/meteor/client/views/room/contextualBar/MessageSearchTab/components/MessageSearch.spec.tsxapps/meteor/client/views/room/contextualBar/Discussions/DiscussionsList.stories.tsx
📚 Learning: 2025-12-10T21:00:54.909Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37091
File: ee/packages/abac/jest.config.ts:4-7
Timestamp: 2025-12-10T21:00:54.909Z
Learning: Rocket.Chat monorepo: Jest testMatch pattern '<rootDir>/src/**/*.spec.(ts|js|mjs)' is valid in this repo and used across multiple packages (e.g., packages/tools, ee/packages/omnichannel-services). Do not flag it as invalid in future reviews.
Applied to files:
apps/meteor/client/views/room/MessageList/MessageList.spec.tsxapps/meteor/client/views/room/contextualBar/Discussions/DiscussionsList.spec.tsxapps/meteor/client/views/room/contextualBar/MessageSearchTab/components/MessageSearch.spec.tsxapps/meteor/client/components/VirtualList/VirtualList.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `expect` matchers for assertions (`toEqual`, `toContain`, `toBeTruthy`, `toHaveLength`, etc.) instead of `assert` statements in Playwright tests
Applied to files:
apps/meteor/client/views/room/MessageList/MessageList.spec.tsxapps/meteor/client/views/room/contextualBar/Discussions/DiscussionsList.spec.tsxapps/meteor/client/views/room/contextualBar/MessageSearchTab/components/MessageSearch.spec.tsxapps/meteor/client/components/VirtualList/VirtualList.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Utilize Playwright fixtures (`test`, `page`, `expect`) for consistency in test files
Applied to files:
apps/meteor/client/views/room/MessageList/MessageList.spec.tsxapps/meteor/client/views/room/contextualBar/Discussions/DiscussionsList.spec.tsxapps/meteor/client/views/room/contextualBar/MessageSearchTab/components/MessageSearch.spec.tsxapps/meteor/client/components/VirtualList/VirtualList.spec.tsx
📚 Learning: 2025-12-16T17:29:45.163Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37834
File: apps/meteor/tests/e2e/page-objects/fragments/admin-flextab-emoji.ts:12-22
Timestamp: 2025-12-16T17:29:45.163Z
Learning: In page object files under `apps/meteor/tests/e2e/page-objects/`, always import `expect` from `../../utils/test` (Playwright's async expect), not from Jest. Jest's `expect` has a synchronous signature and will cause TypeScript errors when used with web-first assertions like `toBeVisible()`.
Applied to files:
apps/meteor/client/views/room/MessageList/MessageList.spec.tsx
📚 Learning: 2026-03-11T22:04:20.529Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 39545
File: apps/meteor/client/views/room/body/hooks/useHasNewMessages.ts:59-61
Timestamp: 2026-03-11T22:04:20.529Z
Learning: In `apps/meteor/client/views/room/body/hooks/useHasNewMessages.ts`, the `msg.u._id === uid` early-return in the `streamNewMessage` handler is intentional: the "New messages" indicator is designed to notify about messages from other users only. Self-sent messages — including those sent from a different session/device — are always skipped, by design. Do not flag this as a multi-session regression.
Applied to files:
apps/meteor/client/views/room/MessageList/MessageList.spec.tsxapps/meteor/client/views/room/contextualBar/MessageSearchTab/components/MessageSearch.tsxapps/meteor/client/views/room/contextualBar/MessageSearchTab/hooks/useMessageSearchQuery.tsapps/meteor/client/views/room/contextualBar/MessageSearchTab/MessageSearchTab.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Prefer web-first assertions (`toBeVisible`, `toHaveText`, etc.) in Playwright tests
Applied to files:
apps/meteor/client/views/room/MessageList/MessageList.spec.tsxapps/meteor/client/views/room/contextualBar/MessageSearchTab/components/MessageSearch.spec.tsxapps/meteor/client/components/VirtualList/VirtualList.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
apps/meteor/client/views/room/MessageList/MessageList.spec.tsxapps/meteor/client/views/room/contextualBar/Discussions/DiscussionsList.spec.tsxapps/meteor/client/views/room/contextualBar/MessageSearchTab/components/MessageSearch.spec.tsx
📚 Learning: 2026-03-06T18:10:15.268Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/code/CodeBlock.spec.tsx:47-68
Timestamp: 2026-03-06T18:10:15.268Z
Learning: In tests (especially those using testing-library/dom/jsdom) for Rocket.Chat components, the HTML <code> element has an implicit ARIA role of 'code'. Therefore, screen.getByRole('code') or screen.findByRole('code') will locate <code> elements even without a role attribute. Do not flag findByRole('code') as invalid in reviews; prefer using the implicit role instead of adding role="code" unless necessary for accessibility.
Applied to files:
apps/meteor/client/views/room/MessageList/MessageList.spec.tsxapps/meteor/client/views/room/contextualBar/Discussions/DiscussionsList.spec.tsxapps/meteor/client/views/room/contextualBar/MessageSearchTab/components/MessageSearch.spec.tsxapps/meteor/client/components/VirtualList/VirtualList.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Group related tests in the same file
Applied to files:
apps/meteor/client/views/room/contextualBar/Discussions/DiscussionsList.spec.tsxapps/meteor/client/views/room/contextualBar/MessageSearchTab/components/MessageSearch.spec.tsxapps/meteor/client/components/VirtualList/VirtualList.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Maintain test isolation between test cases in Playwright tests
Applied to files:
apps/meteor/client/views/room/contextualBar/Discussions/DiscussionsList.spec.tsxapps/meteor/client/views/room/contextualBar/MessageSearchTab/components/MessageSearch.spec.tsxapps/meteor/client/components/VirtualList/VirtualList.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `test.step()` for complex test scenarios to improve organization in Playwright tests
Applied to files:
apps/meteor/client/views/room/contextualBar/Discussions/DiscussionsList.spec.tsxapps/meteor/client/components/VirtualList/VirtualList.spec.tsx
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to **/*.spec.ts : Use descriptive test names that clearly communicate expected behavior in Playwright tests
Applied to files:
apps/meteor/client/views/room/contextualBar/MessageSearchTab/components/MessageSearch.spec.tsx
📚 Learning: 2026-05-26T19:18:05.882Z
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 40644
File: apps/meteor/client/views/room/contextualBar/Threads/components/ThreadMessageList.tsx:145-147
Timestamp: 2026-05-26T19:18:05.882Z
Learning: In `apps/meteor/client/views/room/contextualBar/Threads/components/ThreadMessageList.tsx`, the `clearMsgJumpParam` cleanup in the timed effect (around line 145) intentionally clears the `msg` query parameter only when the target message IS found in `messages` (thread replies) and is not `mainMessage._id`. This is correct behavior: if `msgJumpParam` refers to a non-reply message (e.g., a main channel message), the main message list handles cleanup, so `ThreadMessageList` must not clear the param in that case.
Applied to files:
apps/meteor/client/views/room/contextualBar/MessageSearchTab/components/MessageSearch.tsxapps/meteor/client/views/room/contextualBar/MessageSearchTab/hooks/useMessageSearchQuery.tsapps/meteor/client/views/room/contextualBar/MessageSearchTab/MessageSearchTab.tsx
📚 Learning: 2026-04-28T14:08:46.920Z
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 40105
File: apps/meteor/client/views/room/MessageList/hooks/useTryToJumpToMessage.ts:54-67
Timestamp: 2026-04-28T14:08:46.920Z
Learning: In `apps/meteor/client/views/room/MessageList/hooks/useTryToJumpToMessage.ts`, setting `isJumpingToMessage.current = true` before the guard clauses (RoomHistoryManager.isLoading check, message not found check) is intentional. The flag means "a jump is pending/in progress" and must stay `true` through all intermediate early-return paths (loading, unresolved message, etc.) so that downstream scroll and load behavior is suppressed while waiting for the jump conditions to be satisfied. Do not flag this as a "flag stuck true" bug.
Applied to files:
apps/meteor/client/views/room/contextualBar/MessageSearchTab/components/MessageSearch.tsxapps/meteor/client/views/room/contextualBar/MessageSearchTab/hooks/useMessageSearchQuery.tsapps/meteor/client/views/room/contextualBar/MessageSearchTab/MessageSearchTab.tsx
📚 Learning: 2026-04-29T20:06:34.862Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 40268
File: apps/meteor/client/startup/incomingMessages.ts:21-25
Timestamp: 2026-04-29T20:06:34.862Z
Learning: In `apps/meteor/client/startup/incomingMessages.ts`, the `Messages.state.update` predicate that strips `ignored` from records when `'ignored' in sub` is false (i.e., the subscription update has no `ignored` field) is intentional. Absence of `ignored` in a `subscriptions-changed` event means the user's ignore list is empty/reset, so clearing all existing `ignored` flags on messages for that room is the correct behavior. Do not flag this as an unintentional ignored-state reset on unrelated subscription updates.
Applied to files:
apps/meteor/client/views/room/contextualBar/MessageSearchTab/components/MessageSearch.tsxapps/meteor/client/views/room/contextualBar/MessageSearchTab/hooks/useMessageSearchQuery.tsapps/meteor/client/views/room/contextualBar/MessageSearchTab/MessageSearchTab.tsx
📚 Learning: 2026-05-05T16:51:36.461Z
Learnt from: nazabucciarelli
Repo: RocketChat/Rocket.Chat PR: 40306
File: apps/meteor/client/views/room/MessageList/hooks/useMessageBody.tsx:18-30
Timestamp: 2026-05-05T16:51:36.461Z
Learning: In Rocket.Chat, the `useMaxMarkdownParseLength` hook (in `apps/meteor/client/components/message/hooks/useMaxMarkdownParseLength.ts`) already normalizes values `<= 0` (or missing/non-numeric) to `Infinity`. Consumers like `useMessageBody` and `useNormalizedMessage` can safely use `message.msg.length > maxMarkdownParseLength` without an extra `maxMarkdownParseLength > 0` guard, because `length > Infinity` is always `false`.
Applied to files:
apps/meteor/client/views/room/contextualBar/MessageSearchTab/hooks/useMessageSearchQuery.ts
📚 Learning: 2026-02-25T20:10:16.987Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 38913
File: packages/ddp-client/src/legacy/types/SDKLegacy.ts:34-34
Timestamp: 2026-02-25T20:10:16.987Z
Learning: In the RocketChat/Rocket.Chat monorepo, packages/ddp-client and apps/meteor do not use TypeScript project references. Module augmentations in apps/meteor (e.g., declare module 'rocket.chat/rest-typings') are not visible when compiling packages/ddp-client in isolation, which is why legacy SDK methods that depend on OperationResult types for OpenAPI-migrated endpoints must remain commented out.
Applied to files:
apps/meteor/client/views/room/contextualBar/MessageSearchTab/hooks/useMessageSearchQuery.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,spec.ts} : Follow Page Object Model pattern consistently in Playwright tests
Applied to files:
apps/meteor/client/components/VirtualList/VirtualList.spec.tsx
📚 Learning: 2025-11-19T18:20:07.720Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37419
File: packages/i18n/src/locales/en.i18n.json:918-921
Timestamp: 2025-11-19T18:20:07.720Z
Learning: Repo: RocketChat/Rocket.Chat — i18n/formatting
Learning: This repository uses a custom message formatting parser in UI blocks/messages; do not assume standard Markdown rules. For keys like Call_ended_bold, Call_not_answered_bold, Call_failed_bold, and Call_transferred_bold in packages/i18n/src/locales/en.i18n.json, retain the existing single-asterisk emphasis unless maintainers request otherwise.
Applied to files:
apps/meteor/client/views/room/contextualBar/MessageSearchTab/MessageSearchTab.tsx
📚 Learning: 2026-05-06T20:49:27.193Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 40186
File: packages/apps-engine/src/definition/externalComponent/IExternalComponentRoomInfo.ts:15-15
Timestamp: 2026-05-06T20:49:27.193Z
Learning: In the RocketChat/Rocket.Chat repository, the ExternalComponents feature (related to IExternalComponentRoomInfo, IExternalComponentUserInfo, and IExternalComponentState in packages/apps-engine/src/definition/externalComponent/) is deprecated and unreachable. Do not flag performance or API design concerns for code in this feature area.
Applied to files:
apps/meteor/client/views/room/contextualBar/MessageSearchTab/MessageSearchTab.tsx
🔇 Additional comments (16)
apps/meteor/client/views/room/MessageList/MessageList.spec.tsx (1)
21-21: LGTM!apps/meteor/client/components/VirtualList/index.ts (1)
1-1: LGTM!apps/meteor/client/components/VirtualList/VirtualList.tsx (1)
1-110: LGTM!apps/meteor/client/components/VirtualList/VirtualList.spec.tsx (1)
1-217: LGTM!apps/meteor/client/views/room/contextualBar/Discussions/DiscussionsList.stories.tsx (1)
4-4: LGTM!Also applies to: 8-13, 23-23
apps/meteor/client/views/room/contextualBar/Discussions/DiscussionsList.spec.tsx (1)
2-2: LGTM!Also applies to: 4-7, 16-117
apps/meteor/client/views/room/contextualBar/Discussions/components/DiscussionsListItem.stories.tsx (1)
8-9: LGTM!apps/meteor/client/views/room/contextualBar/Discussions/components/DiscussionsListItem.tsx (1)
44-44: LGTM!apps/meteor/client/views/room/contextualBar/MessageSearchTab/hooks/useMessageSearchQuery.ts (1)
1-42: LGTM!apps/meteor/client/views/room/contextualBar/MessageSearchTab/components/MessageSearch.tsx (1)
1-93: LGTM!apps/meteor/client/views/room/contextualBar/MessageSearchTab/components/MessageSearch.spec.tsx (1)
1-110: LGTM!apps/meteor/client/views/room/contextualBar/MessageSearchTab/MessageSearchTab.tsx (1)
1-67: LGTM!apps/meteor/client/views/room/contextualBar/Discussions/DiscussionsList.tsx (3)
3-3: LGTM!Also applies to: 15-15, 22-22
28-28: LGTM!
56-62: LGTM!apps/meteor/client/views/room/contextualBar/Discussions/DiscussionsListContextBar.tsx (1)
47-47: LGTM!
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
srijnabhargav
force-pushed
the
feat/tanstack-message-search
branch
from
310c396 to
a73e172
Compare
MartinSchoeler
requested changes
Jun 11, 2026
MartinSchoeler
left a comment
MartinSchoeler
left a comment
Member
There was a problem hiding this comment.
The scroll bar does not show up in the search message list for me, it seems the parents are stretching to the height of the list. Check if any of the parent's aren't missing a height attribute
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #40844 +/- ##
===========================================
- Coverage 70.19% 70.03% -0.17%
===========================================
Files 3340 3370 +30
Lines 123638 129375 +5737
Branches 22055 22552 +497
===========================================
+ Hits 86789 90608 +3819
- Misses 33507 35499 +1992
+ Partials 3342 3268 -74
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
MartinSchoeler
requested changes
Jun 11, 2026
coderabbitai
Bot
removed
the
type: feature
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/meteor/client/views/room/contextualBar/MessageSearchTab/hooks/useMessageSearchQuery.ts (1)
29-35: Clarify paging semantics: client is using cumulative “top-N” (not offset pages), soselect-last-page is consistent
getNextPageParamincreaseslimitcumulatively (e.g., 10 → 20 → 30), so each query asks for the first N results again.- On the server,
DefaultProvider.search()forwards onlypayload.limittomessageSearch(nooffset), andparseMessageSearchQueryapplies Mongoskip: offsetwithoffsetdefaulting to0andlimitto the provided value—so every call returnsskip(0).limit(limit)results (top-N).- Given that each page already contains results from the beginning up to N,
select: pages.at(-1)returning only the last page avoids duplicates (flattening would repeat items).itemCount: items.length >= limit ? items.length + 1 : items.lengthmatches the “full page means maybe more” behavior for this top-N approach.Optional: if you want real incremental infinite-scroll (no re-fetching earlier results), add end-to-end offset/cursor paging support (client payload → provider forwarding → Mongo query) and update
getNextPageParamto advance byoffsetinstead of cumulatively increasinglimit.🤖 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 `@apps/meteor/client/views/room/contextualBar/MessageSearchTab/hooks/useMessageSearchQuery.ts` around lines 29 - 35, The paging is using a cumulative "top-N" approach rather than offsets, so update the client logic to make this explicit: keep getNextPageParam in useMessageSearchQuery to continue increasing the requested limit cumulatively (e.g., 10→20→30) and ensure the consumer of the query (the select that picks pages.at(-1)) continues to return only the last page to avoid duplicates; also confirm itemCount logic (items.length >= limit ? items.length + 1 : items.length) matches the "full page means maybe more" behavior. If you instead want true incremental paging, implement end-to-end offset/cursor passing (client payload → DefaultProvider.search forwarding → parseMessageSearchQuery applying skip) and change getNextPageParam to advance by offset rather than increasing limit.
🤖 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
`@apps/meteor/client/views/room/contextualBar/MessageSearchTab/hooks/useMessageSearchQuery.ts`:
- Around line 23-26: The return value from the hook useMessageSearchQuery
currently sets itemCount to items.length + 1 when items.length >= limit which
misrepresents total results; change the hook to return a clear pair such as {
items, itemCount: items.length, hasMore: items.length >= limit } (or rename
itemCount to loadedCount and add hasMore) and update consumers
(MessageSearchTab.tsx -> ResultsLiveRegion and PaginatedVirtualList) to use
itemCount/loadedCount for announced/virtualized counts and use hasMore to
indicate "more available" UI/ARIA cues instead of relying on a +1 heuristic.
In
`@apps/meteor/client/views/room/contextualBar/MessageSearchTab/MessageSearchTab.tsx`:
- Around line 30-31: The screen reader announces data.itemCount (from
useMessageSearchQuery) which may be a "+1 if more" heuristic; change
MessageSearchTab to pass the actual number of loaded items to ResultsLiveRegion
instead of the heuristic value and also pass a boolean flag indicating "more
available" (e.g., compute loadedCount = data?.items?.length ?? 0 and hasMore =
(data?.itemCount ?? 0) > loadedCount), then update ResultsLiveRegion usage to
announce "X results, more available" or similar based on loadedCount and
hasMore; reference useMessageSearchQuery, data.itemCount, ResultsLiveRegion and
the local itemCount variable to locate where to adjust.
---
Nitpick comments:
In
`@apps/meteor/client/views/room/contextualBar/MessageSearchTab/hooks/useMessageSearchQuery.ts`:
- Around line 29-35: The paging is using a cumulative "top-N" approach rather
than offsets, so update the client logic to make this explicit: keep
getNextPageParam in useMessageSearchQuery to continue increasing the requested
limit cumulatively (e.g., 10→20→30) and ensure the consumer of the query (the
select that picks pages.at(-1)) continues to return only the last page to avoid
duplicates; also confirm itemCount logic (items.length >= limit ? items.length +
1 : items.length) matches the "full page means maybe more" behavior. If you
instead want true incremental paging, implement end-to-end offset/cursor passing
(client payload → DefaultProvider.search forwarding → parseMessageSearchQuery
applying skip) and change getNextPageParam to advance by offset rather than
increasing limit.
🪄 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: e9966c89-fd2d-4575-bb7e-64d0a2e674e1
📒 Files selected for processing (4)
apps/meteor/client/views/room/contextualBar/MessageSearchTab/MessageSearchTab.tsxapps/meteor/client/views/room/contextualBar/MessageSearchTab/components/MessageSearch.spec.tsxapps/meteor/client/views/room/contextualBar/MessageSearchTab/components/MessageSearch.tsxapps/meteor/client/views/room/contextualBar/MessageSearchTab/hooks/useMessageSearchQuery.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/meteor/client/views/room/contextualBar/MessageSearchTab/components/MessageSearch.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: cubic · AI code reviewer
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/client/views/room/contextualBar/MessageSearchTab/components/MessageSearch.spec.tsxapps/meteor/client/views/room/contextualBar/MessageSearchTab/hooks/useMessageSearchQuery.tsapps/meteor/client/views/room/contextualBar/MessageSearchTab/MessageSearchTab.tsx
🧠 Learnings (7)
📚 Learning: 2026-03-06T18:10:15.268Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/code/CodeBlock.spec.tsx:47-68
Timestamp: 2026-03-06T18:10:15.268Z
Learning: In tests (especially those using testing-library/dom/jsdom) for Rocket.Chat components, the HTML <code> element has an implicit ARIA role of 'code'. Therefore, screen.getByRole('code') or screen.findByRole('code') will locate <code> elements even without a role attribute. Do not flag findByRole('code') as invalid in reviews; prefer using the implicit role instead of adding role="code" unless necessary for accessibility.
Applied to files:
apps/meteor/client/views/room/contextualBar/MessageSearchTab/components/MessageSearch.spec.tsx
📚 Learning: 2026-03-27T14:52:56.865Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39892
File: apps/meteor/client/views/room/contextualBar/Threads/Thread.tsx:150-155
Timestamp: 2026-03-27T14:52:56.865Z
Learning: In Rocket.Chat, there are two different `ModalBackdrop` components with different prop APIs. During review, confirm the import source: (1) `rocket.chat/fuselage` `ModalBackdrop` uses `ModalBackdropProps` based on `BoxProps` (so it supports `onClick` and other Box/DOM props) and does not have an `onDismiss` prop; (2) `rocket.chat/ui-client` `ModalBackdrop` uses a narrower props interface like `{ children?: ReactNode; onDismiss?: () => void }` and handles Escape keypress and outside mouse-up, and it does not forward arbitrary DOM props such as `onClick`. Flag mismatched props (e.g., `onDismiss` passed to the fuselage component or `onClick` passed to the ui-client component) and ensure the usage matches the correct component being imported.
Applied to files:
apps/meteor/client/views/room/contextualBar/MessageSearchTab/components/MessageSearch.spec.tsxapps/meteor/client/views/room/contextualBar/MessageSearchTab/MessageSearchTab.tsx
📚 Learning: 2026-05-06T12:21:44.083Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 40256
File: apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx:121-149
Timestamp: 2026-05-06T12:21:44.083Z
Learning: Field wrappers in rocket.chat/fuselage-forms (Field, FieldLabel, FieldRow, FieldError, FieldHint) auto-create htmlFor/id associations, aria-describedby, and role="alert" for errors. Do not manually set htmlFor, id, aria-describedby, or role attributes when using these wrappers. This automatic wiring does not apply to plain rocket.chat/fuselage components, which require explicit ID wiring per the accessibility docs. In code reviews, prefer using fuselage-forms wrappers for form fields and verify there is no unnecessary manual ID/aria wiring in files that use these wrappers. If a component uses plain fuselage components, ensure proper id wiring as per docs.
Applied to files:
apps/meteor/client/views/room/contextualBar/MessageSearchTab/components/MessageSearch.spec.tsxapps/meteor/client/views/room/contextualBar/MessageSearchTab/hooks/useMessageSearchQuery.tsapps/meteor/client/views/room/contextualBar/MessageSearchTab/MessageSearchTab.tsx
📚 Learning: 2026-02-10T16:32:42.586Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38528
File: apps/meteor/client/startup/roles.ts:14-14
Timestamp: 2026-02-10T16:32:42.586Z
Learning: In Rocket.Chat's Meteor client code, DDP streams use EJSON and Date fields arrive as Date objects; do not manually construct new Date() in stream handlers (for example, in sdk.stream()). Only REST API responses return plain JSON where dates are strings, so implement explicit conversion there if needed. Apply this guidance to all TypeScript files under apps/meteor/client to ensure consistent date handling in DDP streams and REST responses.
Applied to files:
apps/meteor/client/views/room/contextualBar/MessageSearchTab/hooks/useMessageSearchQuery.ts
📚 Learning: 2026-05-11T20:30:35.265Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 40480
File: apps/meteor/client/meteor/startup/accounts.ts:59-61
Timestamp: 2026-05-11T20:30:35.265Z
Learning: In Rocket.Chat’s Meteor client code, when calling `dispatchToastMessage` with `{ type: 'error' }`, pass the raw caught error object as `message` without manual normalization. `dispatchToastMessage` is designed to accept `message: unknown` for error toasts, so avoid converting errors to strings (e.g., `String(error)`) or extracting `error.message` before passing them.
Applied to files:
apps/meteor/client/views/room/contextualBar/MessageSearchTab/hooks/useMessageSearchQuery.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In the Rocket.Chat repository, do not reference Biome lint rules in code review feedback. Biome is not used even if biome.json exists; only reference Biome rules if there is explicit, project-wide usage documented. For TypeScript files, review lint implications without Biome guidance unless the project enables Biome rules.
Applied to files:
apps/meteor/client/views/room/contextualBar/MessageSearchTab/hooks/useMessageSearchQuery.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.
Applied to files:
apps/meteor/client/views/room/contextualBar/MessageSearchTab/hooks/useMessageSearchQuery.ts
🔇 Additional comments (3)
apps/meteor/client/views/room/contextualBar/MessageSearchTab/components/MessageSearch.spec.tsx (1)
1-124: LGTM!apps/meteor/client/views/room/contextualBar/MessageSearchTab/MessageSearchTab.tsx (1)
51-52: LGTM!apps/meteor/client/views/room/contextualBar/MessageSearchTab/hooks/useMessageSearchQuery.ts (1)
36-36: Fix:selectshould keeppages.at(-1)because paging is cumulative (limit increases, offset stays 0).
useMessageSearchQueryonly passes{ limit, searchAll }(nooffset). On the server,messageSearchusesparseMessageSearchQuerywhereskipdefaults tooffset(default0) andlimitis set from the passedlimit, so each next page returns “first N results”. Takingpages.at(-1)returns the full accumulated list; flattening would duplicate earlier items.> Likely an incorrect or invalid review comment.
MartinSchoeler
requested changes
Jun 11, 2026
coderabbitai
Bot
added
the
type: feature
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes
This PR migrates
MessageSearchfromreact-virtuosoto the sharedPaginatedVirtualListcomponent introduced by the Virtua foundation work.Changes include:
react-virtuosoimplementation withPaginatedVirtualListuseMessageSearchQuerytouseInfiniteQuerywith{ items, itemCount }for paginationMessageSearchwith the same consumer pattern used inDiscussionsListandThreadListMessageSearchTabThe goal is to keep
MessageSearchconsistent with the new virtualization approach while keeping the scope limited to MessageSearch-specific changes.Issue(s)
Part of the Virtua migration work for the contextual bar lists.
Further comments
This PR should be merged after the Virtua foundation work (
PaginatedVirtualListandDiscussionsListmigration), #39394, as it reuses the shared virtualization infrastructure introduced there and only contains MessageSearch-specific changes.Test plan
yarn .testunit:jest --testPathPatterns='contextualBar/MessageSearchTab'yarn typecheckinapps/meteorworking video (no regressions):
Screen.Recording.2026-03-23.at.12.48.18.AM.mov
Summary by CodeRabbit