Conversation
Pages without filter/sort children (Members, Committees) rendered no vertical gap below the SearchBar in mobile viewport, while pages with filters (Chapters, Projects) had mt-2 mb-4 spacing from the mobile filter row. Move a base mb-4 onto the search wrapper (reset at md) so all pages share the same bottom spacing, and compensate the filter row's top margin. Fixes OWASP#5148 Signed-off-by: Vansh Saxena <cyberblink0@gmail.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review. Summary by CodeRabbit
WalkthroughThe search layout now applies consistent responsive spacing. Desktop adds bottom margin below the search bar, while medium screens remove it. Mobile controls use a reduced negative top margin. ChangesSearch spacing
Estimated code review effort: 1 (Trivial) | ~3 minutes Severity of issue fixed: Low Merge Risk: ⚪ Minimal · up to Search layouts now use consistent mobile spacing below the search control while retaining the prior medium-and-up layout. No current merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
There was a problem hiding this comment.
1 issue found across 1 file
Confidence score: 5/5
frontend/src/components/SearchPageLayout.tsxmay leave inconsistent spacing below Search between the two page types because themobile.filter-rowcompensation does not fully align with the wrapper’s newmb-4; verify both mobile layouts and adjust the margin if needed.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="frontend/src/components/SearchPageLayout.tsx">
<violation number="1" location="frontend/src/components/SearchPageLayout.tsx:101">
P3: The mobile.filter-row compensation (`mt-2` → `-mt-1`) leaves the gap below Search inconsistent across the two page types this PR aims to align. With the search wrapper now at `mb-4` (16px), the filter row's `-mt-1` (-4px) yields a 12px search-to-filter gap, while non-filtered pages (Members, Committees) get a 16px search-to-content gap. The prior single-space was 8px (`mt-2`). `-mt-1` matches neither the original 8px (which would need `-mt-2`) nor the non-filtered 16px (which would need `mt-0`), so filtered pages are still spaced differently from non-filtered ones after the fix.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| {(filterChildren || (inlineSort && sortChildren)) && ( | ||
| <div | ||
| className={`mx-auto mt-2 mb-4 flex w-full max-w-md items-stretch md:hidden ${ | ||
| className={`mx-auto -mt-1 mb-4 flex w-full max-w-md items-stretch md:hidden ${ |
There was a problem hiding this comment.
P3: The mobile.filter-row compensation (mt-2 → -mt-1) leaves the gap below Search inconsistent across the two page types this PR aims to align. With the search wrapper now at mb-4 (16px), the filter row's -mt-1 (-4px) yields a 12px search-to-filter gap, while non-filtered pages (Members, Committees) get a 16px search-to-content gap. The prior single-space was 8px (mt-2). -mt-1 matches neither the original 8px (which would need -mt-2) nor the non-filtered 16px (which would need mt-0), so filtered pages are still spaced differently from non-filtered ones after the fix.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/components/SearchPageLayout.tsx, line 101:
<comment>The mobile.filter-row compensation (`mt-2` → `-mt-1`) leaves the gap below Search inconsistent across the two page types this PR aims to align. With the search wrapper now at `mb-4` (16px), the filter row's `-mt-1` (-4px) yields a 12px search-to-filter gap, while non-filtered pages (Members, Committees) get a 16px search-to-content gap. The prior single-space was 8px (`mt-2`). `-mt-1` matches neither the original 8px (which would need `-mt-2`) nor the non-filtered 16px (which would need `mt-0`), so filtered pages are still spaced differently from non-filtered ones after the fix.</comment>
<file context>
@@ -98,7 +98,7 @@ const SearchPageLayout = ({
{(filterChildren || (inlineSort && sortChildren)) && (
<div
- className={`mx-auto mt-2 mb-4 flex w-full max-w-md items-stretch md:hidden ${
+ className={`mx-auto -mt-1 mb-4 flex w-full max-w-md items-stretch md:hidden ${
inlineSort ? 'gap-0' : 'justify-between gap-4'
}`}
</file context>
Proposed change
Fixes inconsistent vertical spacing below the Search component in mobile viewport.
Pages rendered via
SearchPageLayoutwithoutfilterChildren/sortChildren(Members, Committees) had no gap between the search bar and content, while pages with filters (Chapters, Projects) got spacing from the mobile filter row.mb-4to the search-bar wrapper (reset atmd:)mt-2→-mt-1) so filtered pages don't double-spaceResolves #5148
Checklist
tsc --noEmitpasses locally