Fix mobile navigation and project dropdown#900
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR makes the navbar stick to the top, adds a mobile submit-contribution action in the sidebar, and replaces the accepted-project select in the contribution form with a custom dropdown that supports keyboard and outside-click handling. ChangesFrontend UI updates
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@frontend/src/components/Navbar.svelte`:
- Line 127: The Navbar header’s sticky positioning is ineffective because it
sits outside the scrolling container, so `sticky top-0` in `Navbar.svelte` does
not affect the actual scroll behavior. Update the layout so the element that
actually scrolls (the `main.app-main` container or its ancestor structure) is
the one providing the sticky context, or move the header into that scrolling
hierarchy; use the `Navbar` header markup and the `main.app-main` layout
relationship to locate the fix.
In
`@frontend/src/components/portal/submit-contribution/SubmitContribution.svelte`:
- Line 1521: The linked-project-panel z-index is declared twice with the same
value, so remove the redundant mobile-only override and keep the single source
of truth in the SubmitContribution component. Update the styling around
linked-project-panel by relying on the existing Tailwind z-20 on the panel
element and delete the duplicate .linked-project-panel z-index rule from the
mobile styles so there is no repeated declaration.
- Around line 1567-1622: The custom listbox in SubmitContribution.svelte
currently only supports mouse/click selection on the project dropdown buttons
and lacks standard keyboard interaction. Update the project dropdown behavior
around showProjectDropdown, selectProject, and the listbox/options markup so
users can move through acceptedProjects with arrow keys, activate a project with
Enter/Space, and close the menu with Escape while keeping focus handling
consistent. Preserve the existing button-based structure, but add the missing
keyboard event handling and focus management for accessibility.
In `@frontend/src/components/Sidebar.svelte`:
- Around line 118-138: `handleSubmitContribution` duplicates the auth redirect
flow already implemented in `navigate()`, including `setConnectWalletIntent`,
`sessionStorage.redirectAfterLogin`, auth-button clicking, and the mobile close
behavior. Refactor `handleSubmitContribution` to reuse `navigate()` for the
shared authenticated/unauthenticated branching and close logic, and extend
`navigate()` to accept a `ctaId` option so it can handle both `'protected_nav'`
and `'submit_contribution'`. Keep the `trackEvent('submit_contribution_click',
...)` call in `handleSubmitContribution`, but let `navigate()` own the redirect
mechanics.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 9eedaafa-5842-48bf-9ae3-34a6c4afcee0
📒 Files selected for processing (3)
frontend/src/components/Navbar.sveltefrontend/src/components/Sidebar.sveltefrontend/src/components/portal/submit-contribution/SubmitContribution.svelte
| </script> | ||
|
|
||
| <header class="bg-white relative z-50" style="border-bottom: 1.2px solid #e5e5e6;"> | ||
| <header class="bg-white sticky top-0 z-50 flex-shrink-0" style="border-bottom: 1.2px solid #e5e5e6;"> |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect how Sidebar renders on mobile (overlay vs. inline) and whether body/page scrolls on small screens
rg -n "overflow" frontend/src/components/Sidebar.svelte frontend/src/App.svelte
rg -n "fixed|position: fixed|md:hidden|mobile" frontend/src/components/Sidebar.svelteRepository: genlayer-foundation/points
Length of output: 1904
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== App.svelte around navbar/main =="
sed -n '520,552p' frontend/src/App.svelte
echo
echo "== Navbar.svelte around header =="
sed -n '110,140p' frontend/src/components/Navbar.svelte
echo
echo "== Sidebar mobile shell around overlay/panel =="
sed -n '712,736p' frontend/src/components/Sidebar.svelteRepository: genlayer-foundation/points
Length of output: 3801
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
app = Path("frontend/src/App.svelte").read_text()
nav = Path("frontend/src/components/Navbar.svelte").read_text()
side = Path("frontend/src/components/Sidebar.svelte").read_text()
for name, text in [("App.svelte", app), ("Navbar.svelte", nav), ("Sidebar.svelte", side)]:
print(f"\n== {name}: scroll/position markers ==")
for i, line in enumerate(text.splitlines(), 1):
if any(tok in line for tok in ["overflow", "sticky", "fixed", "Navbar", "main", "h-screen", "overflow-hidden", "overflow-y-auto", "top-0", "top: var(--mobile-navbar-height"]):
print(f"{i}: {line}")
PYRepository: genlayer-foundation/points
Length of output: 2045
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check whether the navbar/header is inside the same flex column as the scrolling main,
# and whether the mobile sidebar panel sits below the navbar height.
rg -n "h-screen flex flex-col|overflow-hidden|overflow-y-auto|sticky top-0|top: var\(--mobile-navbar-height" frontend/src/App.svelte frontend/src/components/Navbar.svelte frontend/src/components/Sidebar.svelteRepository: genlayer-foundation/points
Length of output: 1454
sticky won’t change the scroll behavior here Navbar sits outside the only scrolling container (main.app-main), so this class is effectively a no-op unless the app starts scrolling in a different ancestor. If the mobile fix depends on the header staying pinned, adjust the scroll container/layout instead.
🤖 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 `@frontend/src/components/Navbar.svelte` at line 127, The Navbar header’s
sticky positioning is ineffective because it sits outside the scrolling
container, so `sticky top-0` in `Navbar.svelte` does not affect the actual
scroll behavior. Update the layout so the element that actually scrolls (the
`main.app-main` container or its ancestor structure) is the one providing the
sticky context, or move the header into that scrolling hierarchy; use the
`Navbar` header markup and the `main.app-main` layout relationship to locate the
fix.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@frontend/src/components/Sidebar.svelte`:
- Around line 94-99: The mobile sidebar close logic is duplicated in
`handleProfileClick` and `changeCategory` even though `closeMobileSidebar()`
already encapsulates it. Update those handlers to call `closeMobileSidebar()`
instead of inlining the `window.innerWidth < 768` check, keeping the sidebar
behavior centralized in the `Sidebar.svelte` helper and avoiding repeated logic.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 2ad49ff3-92da-40f6-b6ad-0df86fa439f0
📒 Files selected for processing (2)
frontend/src/components/Sidebar.sveltefrontend/src/components/portal/submit-contribution/SubmitContribution.svelte
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
1 participant
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.
Summary
Validation
Summary by CodeRabbit