fix: marketplace filter dropdown positioning on mobile (#38651)#38899
Open
JatinSharma222 wants to merge 3 commits into
Open
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 |
🦋 Changeset detectedLatest commit: 65d42ae The changes in this PR will be included in the next version bump. This PR includes changesets to release 41 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🔇 Additional comments (1)
WalkthroughThis PR updates marketplace filter layout and dropdown containers to use Box wrappers with responsive sizing and explicit inline absolute positioning for dropdown lists, reorganizes filter rows for breakpoints, and adjusts child width handling to fix dropdown placement on small screens. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
- Separate search bar from filter dropdowns into own row - Wrap each dropdown in position:relative Box to anchor correctly - Switch DropDownListWrapper from fixed to absolute positioning - Filters stay horizontal on all screen sizes with proper gap Fixes RocketChat#38651
JatinSharma222
force-pushed
the
fix/38651-marketplace-mobile-filter-dropdown-positioning
branch
from
ac90900 to
0815c2d
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
apps/meteor/client/views/marketplace/AppsPage/AppsFilters.tsx (1)
65-79: Redundantposition='relative'on wrapperBoxelementsEach
RadioDropDownandCategoryDropDownnow establishes its ownposition: relativecontext internally (via theBox position='relative'wrapper added in this PR). CSSposition: absoluteanchors to the nearest positioned ancestor, soDropDownListWrapperalready anchors correctly to the component's own innerBox. Theposition='relative'on these outer wrapper boxes inAppsFiltershas no effect and can be removed.🛠️ Proposed cleanup
- <Box position='relative' flexGrow={1} flexShrink={1} flexBasis='x0' {...filterBoxProps}> + <Box flexGrow={1} flexShrink={1} flexBasis='x0' {...filterBoxProps}> <RadioDropDown group={freePaidFilterStructure} onSelected={freePaidFilterOnSelected} width='100%' /> </Box>(Apply the same removal to all four wrapper boxes at lines 65, 69, 73, and 77.)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/client/views/marketplace/AppsPage/AppsFilters.tsx` around lines 65 - 79, Remove the redundant position='relative' props from the outer Box wrappers in AppsFilters that wrap RadioDropDown and CategoryDropDown (the four Box instances around RadioDropDown group={freePaidFilterStructure}, RadioDropDown group={statusFilterStructure}, CategoryDropDown, and RadioDropDown group={sortFilterStructure}); each dropdown already provides its own positioned inner Box/DropDownListWrapper, so simply delete the position='relative' attribute on those Box components and keep the rest of their props (flexGrow, flexShrink, flexBasis, {...filterBoxProps}) unchanged.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/meteor/client/views/marketplace/AppsPage/AppsFilters.tsxapps/meteor/client/views/marketplace/components/CategoryFilter/CategoryDropDown.tsxapps/meteor/client/views/marketplace/components/DropDownListWrapper.tsxapps/meteor/client/views/marketplace/components/RadioDropDown/RadioDropDown.tsx
🧰 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/marketplace/AppsPage/AppsFilters.tsxapps/meteor/client/views/marketplace/components/CategoryFilter/CategoryDropDown.tsxapps/meteor/client/views/marketplace/components/DropDownListWrapper.tsxapps/meteor/client/views/marketplace/components/RadioDropDown/RadioDropDown.tsx
⏰ 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 comments (1)
apps/meteor/client/views/marketplace/components/RadioDropDown/RadioDropDown.tsx (1)
31-38: LGTM — Wrapping inBox position='relative'correctly establishes the stacking context forDropDownListWrapper'sposition: absolute; top: 100%.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/meteor/client/views/marketplace/AppsPage/AppsFilters.tsx`:
- Around line 61-63: The child filter row Box (Box with props display='flex'
flexDirection='row' flexWrap='wrap' alignItems='center' mbs={-16}) is using a
negative mbs that cancels the parent column Box's style={{ rowGap: '8px' }},
causing overlap; remove the mbs={-16} or replace it with a positive spacing
token consistent with the design system (e.g., use an existing positive spacing
prop or mbs={16}) so the rowGap on the parent Box and spacing tokens are aligned
and no negative margin is used.
In
`@apps/meteor/client/views/marketplace/components/CategoryFilter/CategoryDropDown.tsx`:
- Around line 36-43: CategoryDropDownAnchor can miss width when callers don't
pass it because it only gets {...props}; make its width explicit like
RadioDropDownAnchor so it always fills the surrounding Box: update the
CategoryDropDown component to render <CategoryDropDownAnchor {...props}
width='100%' ref={reference} onClick={toggleCollapsed as any}
selectedCategoriesCount={selectedCategories.length} /> (i.e., ensure
width='100%' is set on CategoryDropDownAnchor rather than relying on callers to
supply it) so the anchor consistently stretches inside the Box.
In `@apps/meteor/client/views/marketplace/components/DropDownListWrapper.tsx`:
- Around line 6-9: Remove the unused forwardRef wrapper around
DropDownListWrapper and stop swallowing the caller's anchor ref: change the
component signature for CategoryDropDownListWrapper/DropDownListWrapper to
accept an optional anchorRef prop (e.g., anchorRef?: RefObject<HTMLElement>)
instead of a forwarded ref, keep the internal target = useRef<HTMLElement>(null)
for the Box, and call useOutsideClick([target, anchorRef], onClose) so the
anchor is excluded; then update callers (RadioDropDown, CategoryDropDown) to
pass anchorRef={reference} rather than ref={reference}.
---
Nitpick comments:
In `@apps/meteor/client/views/marketplace/AppsPage/AppsFilters.tsx`:
- Around line 65-79: Remove the redundant position='relative' props from the
outer Box wrappers in AppsFilters that wrap RadioDropDown and CategoryDropDown
(the four Box instances around RadioDropDown group={freePaidFilterStructure},
RadioDropDown group={statusFilterStructure}, CategoryDropDown, and RadioDropDown
group={sortFilterStructure}); each dropdown already provides its own positioned
inner Box/DropDownListWrapper, so simply delete the position='relative'
attribute on those Box components and keep the rest of their props (flexGrow,
flexShrink, flexBasis, {...filterBoxProps}) unchanged.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/meteor/client/views/marketplace/AppsPage/AppsFilters.tsx (1)
65-79: Redundantposition='relative'on each filter wrapperBox.Each
RadioDropDownandCategoryDropDownnow internally wraps its content in<Box position='relative'>, which becomes the nearest positioned ancestor forDropDownListWrapper'sposition: absolute. The extraposition='relative'on the outer wrapperBoxinAppsFilters(lines 65, 69, 73, 77) has no effect on dropdown placement and can be removed to simplify the tree.♻️ Proposed refactor
- <Box position='relative' flexGrow={1} flexShrink={1} flexBasis='x0' {...filterBoxProps}> + <Box flexGrow={1} flexShrink={1} flexBasis='x0' {...filterBoxProps}> <RadioDropDown group={freePaidFilterStructure} onSelected={freePaidFilterOnSelected} width='100%' /> </Box> ... - <Box position='relative' flexGrow={1} flexShrink={1} flexBasis='x0' {...filterBoxProps}> + <Box flexGrow={1} flexShrink={1} flexBasis='x0' {...filterBoxProps}> <RadioDropDown group={statusFilterStructure} onSelected={statusFilterOnSelected} width='100%' /> </Box> ...(Apply the same removal to lines 73 and 77.)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/client/views/marketplace/AppsPage/AppsFilters.tsx` around lines 65 - 79, Remove the redundant position='relative' prop from the outer Box wrappers that wrap RadioDropDown and CategoryDropDown in AppsFilters: the dropdown components (RadioDropDown and CategoryDropDown) already include an internal <Box position='relative'> so the outer Box position prop is unnecessary; update the Boxes around freePaidFilterStructure, statusFilterStructure, categories/selectedCategories, and sortFilterStructure (the Box instances that pass filterBoxProps and width='100%') to omit position='relative' while keeping all other props (flexGrow, flexShrink, flexBasis, {...filterBoxProps}) intact.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/meteor/client/views/marketplace/AppsPage/AppsFilters.tsxapps/meteor/client/views/marketplace/components/CategoryFilter/CategoryDropDown.tsxapps/meteor/client/views/marketplace/components/DropDownListWrapper.tsxapps/meteor/client/views/marketplace/components/RadioDropDown/RadioDropDown.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/meteor/client/views/marketplace/components/DropDownListWrapper.tsx
🧰 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/marketplace/components/CategoryFilter/CategoryDropDown.tsxapps/meteor/client/views/marketplace/components/RadioDropDown/RadioDropDown.tsxapps/meteor/client/views/marketplace/AppsPage/AppsFilters.tsx
🧠 Learnings (1)
📚 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/marketplace/AppsPage/AppsFilters.tsx
🔇 Additional comments (1)
apps/meteor/client/views/marketplace/components/RadioDropDown/RadioDropDown.tsx (1)
30-39: LGTM —Box position='relative'correctly anchors the absolute-positioned dropdown list.Explicit
width='100%'onRadioDropDownAnchor(line 32) ensures the anchor always fills the wrapper, consistent with how other dropdown anchors are handled.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/meteor/client/views/marketplace/AppsPage/AppsFilters.tsx`:
- Around line 61-63: The nested Box with filters uses mbs={-16} which conflicts
with the parent Box's rowGap: '8px' and causes overlap; update the filter row
(the inner Box that contains FilterByText and tag chips) to remove the negative
mbs prop or replace it with a positive Fuselage spacing token (e.g., mbs={16} or
an approved token) so spacing is controlled by the parent rowGap consistently;
keep references to the Box component around FilterByText and the setText handler
to locate the element to change.
In
`@apps/meteor/client/views/marketplace/components/CategoryFilter/CategoryDropDown.tsx`:
- Around line 36-43: CategoryDropDownAnchor is not explicitly given
width='100%', so if callers omit width via {...props} the anchor can be narrower
than its Box wrapper; update the JSX to pass width='100%' directly to
CategoryDropDownAnchor (e.g., <CategoryDropDownAnchor ref={reference}
width='100%' onClick={toggleCollapsed as any}
selectedCategoriesCount={selectedCategories.length} {...props} />) so the anchor
always fills the Box; keep the rest of props, ref, onClick, and
selectedCategoriesCount unchanged.
---
Nitpick comments:
In `@apps/meteor/client/views/marketplace/AppsPage/AppsFilters.tsx`:
- Around line 65-79: Remove the redundant position='relative' prop from the
outer Box wrappers that wrap RadioDropDown and CategoryDropDown in AppsFilters:
the dropdown components (RadioDropDown and CategoryDropDown) already include an
internal <Box position='relative'> so the outer Box position prop is
unnecessary; update the Boxes around freePaidFilterStructure,
statusFilterStructure, categories/selectedCategories, and sortFilterStructure
(the Box instances that pass filterBoxProps and width='100%') to omit
position='relative' while keeping all other props (flexGrow, flexShrink,
flexBasis, {...filterBoxProps}) intact.
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.
Description
Fixes #38651
On mobile screen sizes (≤ 768px), the filter dropdowns in the Marketplace page
(Explore, Installed, Premium) were rendering in incorrect positions — dropdowns
would appear under the wrong filter trigger due to a positioning miscalculation.
Root Cause
The
DropDownListWrappercomponent usedusePositionwithposition: fixed,which calculates dropdown position based on the viewport. On mobile, when the
flex container wraps filters across multiple lines, the fixed positioning loses
its reference point and attaches the dropdown to the wrong anchor element.
Changes
DropDownListWrapper.tsx— ReplacedusePosition+fixedpositioningwith
position: absolute+top: 100%RadioDropDown.tsx— Wrapped inposition: relativeBox for isolatedpositioning context
CategoryDropDown.tsx— Same fix as RadioDropDownAppsFilters.tsx— Separated search bar and filters into independent rows;filters stay horizontal on all screen sizes with proper spacing
After Fix
Screen.Recording.2026-02-22.at.7.05.31.PM.mov
Testing Steps
Summary by CodeRabbit
Refactor
Bug Fixes