feat(mobile): Add an app-style bottom navigation bar#125
Open
OffCrazyFreak wants to merge 44 commits into
Open
feat(mobile): Add an app-style bottom navigation bar#125OffCrazyFreak wants to merge 44 commits into
OffCrazyFreak wants to merge 44 commits into
Conversation
…nav item Changes: - Declare --bottom-nav-h, --bottom-nav-gap, --bottom-nav-safe and --bottom-nav-total in globals.css - Register three animatable custom properties and the bottom-nav-compact keyframes - Bind the compaction to a scroll timeline behind @supports and prefers-reduced-motion - Add productsNavItem for /products, which productNavItems only ever reached through filters The bar needs dimensions that Tailwind's spacing scale cannot express here: this project sets --spacing to 0.2rem, so h-16 would render 51px instead of 64px. Keeping the numbers in custom properties also means the page padding and the bar height can never drift apart. Notes: - Custom properties must be registered with @Property or they animate discretely and snap at the halfway point - The reserved space includes the floating variant's gap for every variant, so page padding does not change when the surface style does - Browsers without scroll-driven animations keep the full-size bar, which is the correct fallback since navigation must never become unreachable
Changes: - Add utils/long-press.ts, a framework-free press timer with progress and cancellation - Add hooks/use-long-press.ts, the element-scoped Pointer Events wrapper - Add hooks/use-tab-reentry.ts for scroll-to-top then return-to-position - Extract scrollWindowTo in utils/scroll.ts and route scrollToTop through it - Add bottom-nav-items.ts, the bar's five-item order and its long-press mapping The gesture cannot be built on `contextmenu`, which iOS has not fired on long press since 13.1, so it is a pointerdown timer that cancels once the pointer moves more than 10px. The timer lives outside React because the bar and the product cards own very different pointer streams but need identical timing. Notes: - scrollToTop claimed to honour reduced motion but did not: browsers apply that only to the CSS scroll-behavior property, never to the JS behavior option - Press progress is written to the element as --press-progress rather than held in state, so the ring does not re-render its subtree once per frame - The bar's order is deliberate: the two coming-soon items take the hardest-to-reach outer positions and search sits dead centre
…rfaces Changes: - Add bottom-nav.tsx, the md:hidden shell that resolves the variant and owns activation - Add bottom-nav-item.tsx and bottom-nav-center-item.tsx for the plain and raised cells - Add bottom-nav-indicator.tsx, a layoutId pill that slides between cells - Add bottom-nav-ring.tsx, shared by long-press feedback and list completion - Add use-bottom-nav-pointer.ts, one pointer stream for tap, scrub and long press - Add use-active-list-progress.ts, completion of the most recently touched list - Add bottom-nav-variant.ts and its dev-only switcher, plus the persisted preference - Reset the saved re-entry position when the route changes The three surfaces share one markup tree and differ only by a data-variant attribute, so there is nothing to keep in sync while they are being compared. Cells are buttons rather than links because iOS shows a link-preview popover and a callout on a long-pressed anchor, and -webkit-touch-callout is unreliable as of iOS 26; the crawlable links already ship in the header and the sidebar. Notes: - Activation runs on pointerup, not per-cell clicks: capturing the pointer on the list retargets the click away from the button, so cells handle only keyboard clicks, which arrive with detail 0 - A tap is a zero-distance scrub, so both paths are the same code - Presses starting within 16px of a screen edge are ignored, leaving the OS back-swipe and home gestures alone - Hidden from md up by CSS rather than a media-query hook, so the markup ships in the prerendered HTML and nothing shifts on hydration
Changes: - Add search-sheet.tsx, a full-screen sheet whose field is always mounted - Add search-sheet-recents.tsx with recent queries and the scanner entry point - Add SearchSheetProvider, which reveals the sheet and focuses it in one task - Add recent-search storage helpers and the shared morph layout id - Give SearchBar an inputRef and an onSubmitted callback, plus search input attributes Navigating to /products and then focusing its field cannot raise the keyboard on iOS, which only does so when .focus() runs inside the tap's own task, while a router transition is async. A sheet whose field already exists keeps the focus call synchronous. This is also what iOS 26 now does with its search tab role. Notes: - The sheet hides with visibility, not opacity alone, so a closed sheet leaves the tab order and the accessibility tree - Since a hidden ancestor also blocks focus, open() reveals the container through the DOM and forces a style recalc before focusing, ahead of React committing - Picking a recent query goes through useSearchNavigation rather than writing to the input, which would bypass react-hook-form - The morph is a decorative layer behind the field, so the field itself never unmounts
Changes: - Delete fab-menu.tsx, page-fab.tsx and fab-action.ts, now without callers - Make BackToTopButton desktop-only, at the breakpoint where the bottom nav ends - Point the four former PageFab sites at BackToTopButton directly - Drop `hidden sm:inline-flex` from the three inline create buttons - Offset the install banner and the sonner toaster above the bar The mobile FAB sat at bottom-4 right-4 z-50, exactly where the bar goes, and both of its jobs now live in the bar: the per-page create action became a long press plus a visible inline button, and back-to-top became re-tapping the active tab. So it was redundant with the bar, not merely colliding with it. Notes: - The inline create buttons had to become visible on mobile, since with the FAB gone they were the only remaining tappable path and a long press alone would fail WCAG 2.1.1 - BackToTopButton moved from sm to md, or widths between 640px and 768px would have shown both the bar and the FAB - floating-action-button.tsx stays: BackToTopButton still builds on it
Changes: - Add product-quick-actions.tsx, a drawer with add-to-list, track price and share - Wire useLongPress into ProductItem, and take pressProps on ProductCard - Extract useProductModals, which seeds the by-ean cache before opening a modal - Add shareOrCopy, sharing through the OS sheet and copying the link otherwise - Give /spending a value prop and an Obavijesti me action via ComingSoon's new action slot Holding a product card is where the watchlist shortcut belongs: the context is the product in your hand, so the gesture means one thing everywhere, unlike a tab whose meaning would change per route. Notes: - ProductCard is a Card with role=button, not an anchor, so there is no iOS link-preview to fight here, only the selection callout - The card's own onClick checks hasFired(), or a press that opened the drawer would also navigate on release - The by-ean seeding was already duplicated in two components; the URL-driven modals take no props, so without it they open empty - /digital-cards is left alone: it is a working page, flagged coming-soon only because barcode rendering is still pending, so a teaser state there would be wrong
…e bar Changes: - Replace the full-screen overlay with a panel anchored to the bottom edge - Run it behind the bar at z-44 and pad its content clear of the bar - Use SearchBar's existing block submit and scanning, so Pretraži is disabled until something is typed - Close the sheet once a search or a scan has run - Give the centre cell a Proizvodi label under its raised circle - Drop the recent-search list, its storage and the morph layer, none of which this needs - Grow the active indicator into a disc enclosing both icon and label, and bold the active label The sheet is now the SearchBar configuration the sidebar already uses, so there is no second search field to maintain. Fixing the eslint refs errors at the same time: both press timers are now built on first press rather than during render. Notes: - The field is still always mounted and still hidden by visibility, since that is what lets iOS raise the keyboard from the tap's own task - z-order is backdrop 43, sheet 44, bar 45, dialogs 50
Changes: - Close on pathname change, matching AppSidebar's behaviour - Mark the Kartice long-press flag as a TODO for its launch Navigating away left the sheet open behind the new page, since only submitting a search closed it. Notes: - Submitting from /products only changes the query, not the pathname, which is already covered by onSubmitted - Focus on open needed no work: ModalShell deliberately keeps input autoFocus alive, and both the new-list and new-card forms already carry it on their name field, so a long press lands the cursor ready to type
Changes:
- Add components/custom/modal/sheet-shell.tsx, ModalShell's bottom-sheet counterpart
- Move the search sheet, the product quick actions and the product filters onto it
- Give the search sheet the grab handle, swipe-to-close and a focused field on open
- Scope the Popisi completion ring to a list's own detail page
- Grow both nav rings to enclose icon and label, matching the active disc
- Simplify the search-sheet context now that the shell owns focus on open
Three bottom sheets had grown three different shells. One shared shell means the
grab handle, the swipe-to-close and the focus behaviour are identical everywhere
and only the content differs.
Notes:
- The search sheet passes modal={false}, which drops vaul's scrim and scroll lock so it can sit behind the nav at z-44 with the bar still visible and tappable
- Focus on open is explicit through initialFocusRef rather than relying on a child's autoFocus surviving Radix's focus scope
- The completion ring now reads the list you are actually looking at via useGetShoppingListById, instead of guessing from the most recently updated list
- Trade-off worth knowing: the field is no longer mounted before the tap, so on iOS the cursor lands but the keyboard may need one tap. Android and desktop open it directly
Changes: - Add docs/MOBILE-NAV.md covering the bar, its gestures, and the shared sheet shell - Link it from the docs index Documents the parts that are not obvious from reading the code: why cells are buttons rather than links, why --spacing being 0.2rem breaks every size utility, why viewportFit cover is load-bearing, and where the long-press timing numbers came from. Notes: - Includes the research behind the design decisions, so the next person does not re-litigate the tab count, the teaser cells or the missing haptics - The gotchas section is the part worth reading first
❌ Deploy Preview for disscount failed. Why did it fail? →
|
Contributor
Summary by CodeRabbit
WalkthroughThe PR introduces a mobile bottom navigation with scrubbing, long-press actions, search and filter sheets, safe-area-aware layout, shared sheet primitives, product quick actions, FAB replacements, and comprehensive implementation documentation. ChangesMobile navigation
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant BottomNav
participant SearchSheetProvider
participant SearchSheet
participant SearchBar
User->>BottomNav: Tap or scrub navigation cell
BottomNav->>SearchSheetProvider: Toggle search state
SearchSheetProvider->>SearchSheet: Provide open state and input ref
SearchSheet->>SearchBar: Focus search input
SearchBar->>SearchSheet: Submit query or scanned code
SearchSheet->>SearchSheetProvider: Close on route change
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
⚔️ Resolve merge conflicts
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@docs/MOBILE-NAV.md`:
- Line 40: Update docs/MOBILE-NAV.md lines 40-40 and 41-41: describe icon labels
as visible before scroll compaction rather than always visible, and change the
scrim/WindowScrollFade z-index value from 43 to 40.
In `@frontend/src/components/custom/product/product-card.tsx`:
- Around line 73-78: Update ProductCard’s trailing ProductActionButtons
container to stop pointer-event propagation so interactions with trailing
controls cannot reach the card-level long-press handlers passed by ProductItem.
Preserve the existing card gesture for non-interactive content and keep the
action buttons’ normal click/press behavior intact.
In `@frontend/src/utils/browser/share.ts`:
- Around line 16-23: Update the navigator.share error handling in the share flow
to return "dismissed" only when the rejection is a DOMException whose name is
"AbortError"; return "failed" for all other rejection types so callers can fall
back to the intended failure behavior.
🪄 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: f9839307-bec4-49ad-9d27-826a8cd7897b
📒 Files selected for processing (48)
docs/MOBILE-NAV.mddocs/README.mdfrontend/src/app/(user)/digital-cards/components/create-digital-card-button.tsxfrontend/src/app/(user)/shopping-lists/components/create-shopping-list-button.tsxfrontend/src/app/(user)/spending/page.tsxfrontend/src/app/(user)/watchlist/components/create-discounted-list-button.tsxfrontend/src/app/globals.cssfrontend/src/app/layout.tsxfrontend/src/app/products/components/product-action-buttons.tsxfrontend/src/app/products/components/product-filters-bar.tsxfrontend/src/app/products/components/product-item/product-item.tsxfrontend/src/app/products/components/products-client.tsxfrontend/src/app/products/components/watchlist-action-button.tsxfrontend/src/app/providers/providers.tsxfrontend/src/app/providers/toaster-provider.tsxfrontend/src/components/custom/bottom-nav/bottom-nav-center-item.tsxfrontend/src/components/custom/bottom-nav/bottom-nav-indicator.tsxfrontend/src/components/custom/bottom-nav/bottom-nav-item.tsxfrontend/src/components/custom/bottom-nav/bottom-nav-items.tsfrontend/src/components/custom/bottom-nav/bottom-nav-ring.tsxfrontend/src/components/custom/bottom-nav/bottom-nav-variant-switcher.tsxfrontend/src/components/custom/bottom-nav/bottom-nav-variant.tsfrontend/src/components/custom/bottom-nav/bottom-nav.tsxfrontend/src/components/custom/bottom-nav/use-active-list-progress.tsfrontend/src/components/custom/bottom-nav/use-bottom-nav-pointer.tsfrontend/src/components/custom/common/coming-soon.tsxfrontend/src/components/custom/common/notify-me-button.tsxfrontend/src/components/custom/fab/back-to-top-button.tsxfrontend/src/components/custom/fab/fab-action.tsfrontend/src/components/custom/fab/fab-menu.tsxfrontend/src/components/custom/fab/page-fab.tsxfrontend/src/components/custom/modal/sheet-shell.tsxfrontend/src/components/custom/product/product-card.tsxfrontend/src/components/custom/product/product-quick-actions.tsxfrontend/src/components/custom/pwa/install-banner.tsxfrontend/src/components/custom/search/search-bar.tsxfrontend/src/components/custom/search/search-sheet.tsxfrontend/src/constants/navigation.tsfrontend/src/context/search-sheet-context.tsxfrontend/src/hooks/use-long-press.tsfrontend/src/hooks/use-product-modals.tsfrontend/src/hooks/use-tab-reentry.tsfrontend/src/typings/local-storage.tsfrontend/src/utils/browser/local-storage.tsfrontend/src/utils/browser/share.tsfrontend/src/utils/browser/storage/bottom-nav.tsfrontend/src/utils/long-press.tsfrontend/src/utils/scroll.ts
💤 Files with no reviewable changes (3)
- frontend/src/components/custom/fab/page-fab.tsx
- frontend/src/components/custom/fab/fab-action.ts
- frontend/src/components/custom/fab/fab-menu.tsx
Comment on lines
+73
to
+78
| {...pressProps} | ||
| className={cn( | ||
| "@container shadow-sm hover:shadow-lg transition-shadow", | ||
| onClick && "cursor-pointer", | ||
| // Suppresses the iOS selection callout a long press would otherwise raise. | ||
| pressProps && "select-none [-webkit-touch-callout:none]", |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
git ls-files | grep -F 'frontend/src/components/custom/product/product-card.tsx' || true
echo "== file outline =="
ast-grep outline frontend/src/components/custom/product/product-card.tsx --view compact || true
echo "== relevant section =="
nl -ba frontend/src/components/custom/product/product-card.tsx | sed -n '1,140p'
echo "== search ProductItem and press handlers =="
rg -n "ProductItem|long-press|onLongPress|pressProps|[-webkit-touch-callout|select-none" frontend/src || true
echo "== deterministic bubbling probe from source text =="
python3 - <<'PY'
from pathlib import Path
p = Path('frontend/src/components/custom/product/product-card.tsx')
s = p.read_text()
print("root card uses pressProps:", "{...pressProps}" in s or "{pressProps}" in s)
print("pressProps presence:", "pressProps" in s)
print("trailing container onClick:", 'className="flex shrink-0 items-center justify-between gap-4"' in s and "onClick={stopCardNavigation}" in s[p.find('trailing &&'):p.find('trailing &&') + 250])
print("pointerDown on trailing container present:", "onPointerDown" in s[p.find('trailing &&'):p.find('trailing &&') + 800])
PYRepository: OffCrazyFreak/Disscount
Length of output: 469
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="; ls -l frontend/src/components/custom/product/product-card.ts
echo "== relevant section =="; cat -n frontend/src/components/custom/product/product-card.ts | sed -n '1,140p'
echo "== search ProductItem and press handlers =="; rg -n "ProductItem|long-press|onLongPress|pressProps|[-webkit-touch-callout|select-none" frontend/src || true
echo "== deterministic source checks =="; python3 - <<'PY'
from pathlib import Path
p = Path('frontend/src/components/custom/product/product-card.ts')
s = p.read_text()
idx = s.find('trailing &&')
print("root card uses pressProps:", "{...pressProps}" in s)
print("pressProps presence:", "pressProps" in s)
print("trailing container onClick presence and nearby:", "onClick={stopCardNavigation}" in s[idx:idx+400])
print("pointerDown on trailing container in nearby section:", "onPointerDown" in s[idx:idx+400])
PYRepository: OffCrazyFreak/Disscount
Length of output: 281
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
find frontend -maxdepth 8 -type f -name 'product-card.tsx' -print
echo "== relevant section =="
cat -n frontend/src/components/custom/product/product-card.tsx | sed -n '1,150p'
echo "== search ProductItem and press handlers =="
rg -n "ProductItem|long-press|onLongPress|pressProps|[-webkit-touch-callout|select-none" frontend/src || true
echo "== deterministic source checks =="
python3 - <<'PY'
from pathlib import Path
p = Path('frontend/src/components/custom/product/product-card.tsx')
s = p.read_text()
idx = s.find('trailing &&')
print("file:", p, "exists:", p.exists(), "length:", len(s))
print("root card uses pressProps:", "{...pressProps}" in s)
print("pressProps presence:", "pressProps" in s)
print("trailing container onClick presence and nearby:", "onClick={stopCardNavigation}" in s[idx:idx+400] if idx != -1 else "no trailing && found")
print("pointerDown on trailing container in nearby section:", "onPointerDown" in (s[idx:idx+400] if idx != -1 else ""))
print("section length:", len(s[idx:idx+400]) if idx != -1 else 0)
PYRepository: OffCrazyFreak/Disscount
Length of output: 4830
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== ProductItem and long-press/pressProps usages =="
rg -n 'ProductItem|long-press|onLongPress|onLongPressStart|onLongPressEnd|onPointerDown|pressProps|[-webkit-touch-callout:none|select-none' frontend/src || true
echo "== product-card usage in ProductItem =="
rg -n "ProductCard|trailing=" frontend/src -g '*.tsx' -g '*.ts' || true
echo "== focused file listings =="
find frontend/src -type f \( -name '*product*' -o -name '*long*' \) -print | sortRepository: OffCrazyFreak/Disscount
Length of output: 3422
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== product item =="
cat -n frontend/src/app/products/components/product-item/product-item.tsx | sed -n '1,120p'
echo "== watchlist item =="
cat -n 'frontend/src/app/(user)/watchlist/components/watchlist-item.tsx' | sed -n '1,90p'
echo "== product quick actions =="
cat -n frontend/src/components/custom/product/product-quick-actions.tsx | sed -n '1,180p'
echo "== long press hooks/utils =="
cat -n frontend/src/hooks/use-long-press.ts | sed -n '1,220p'
cat -n frontend/src/utils/long-press.ts | sed -n '1,220p'
echo "== source property checks =="
python3 - <<'PY'
from pathlib import Path
for p in [
Path('frontend/src/hooks/use-long-press.ts'),
Path('frontend/src/utils/long-press.ts'),
Path('frontend/src/app/products/components/product-item/product-item.tsx'),
Path('frontend/src/app/(user)/watchlist/components/watchlist-item.tsx'),
]:
print("---", p)
s = p.read_text()
for term in ["useLongPress", "longPressProps", "onPointerDown", "onClick", "trailing", "ProductCard"]:
print(f"{term}: {term in s}")
PYRepository: OffCrazyFreak/Disscount
Length of output: 15361
Keep trailing controls out of the product-card long-press gesture.
ProductItem passes long-press pointer handlers to ProductCard, which attaches them to the card root. A long press on the trailing ProductActionButtons bubbles there, opens quick actions, and because onPointerUp only cancels the timer, a quick hold/release still triggers the button. Stop pointer propagation in the trailing container, or bind the gesture only to non-interactive card content.
🤖 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/custom/product/product-card.tsx` around lines 73 -
78, Update ProductCard’s trailing ProductActionButtons container to stop
pointer-event propagation so interactions with trailing controls cannot reach
the card-level long-press handlers passed by ProductItem. Preserve the existing
card gesture for non-interactive content and keep the action buttons’ normal
click/press behavior intact.
Comment on lines
+16
to
+23
| if (navigator.share) { | ||
| try { | ||
| await navigator.share(data); | ||
| return "shared"; | ||
| } catch { | ||
| // Dismissing the sheet is a choice, not a failure to fall back from. | ||
| return "dismissed"; | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Repository files matching share.ts:\n'
fd -a 'share\.ts$' . || true
printf '\nInspect target file:\n'
if [ -f frontend/src/utils/browser/share.ts ]; then
nl -ba frontend/src/utils/browser/share.ts
fi
printf '\nSearch for share function usages and response handling:\n'
rg -n "share\\(|dismissed|failed" frontend/src/frontend 2>/dev/null || true
rg -n "share\\(|dismissed|failed|shared" frontend/src 2>/dev/null || true
printf '\nSearch in frontend root with context:\n'
rg -n "share\\(|dismissed|failed|shared" frontend --glob '!**/node_modules/**' 2>/dev/null || trueRepository: OffCrazyFreak/Disscount
Length of output: 314
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Inspect target file:\n'
sed -n '1,220p' frontend/src/utils/browser/share.ts
printf '\nSearch for share function usages and response handling:\n'
rg -n "share\\(|shared|dismissed|failed" frontend/src --glob '!**/node_modules/**' 2>/dev/null || true
printf '\nRead-only parser-style check for navigator.share catch behavior:\n'
python3 - <<'PY'
from pathlib import Path
p = Path("frontend/src/utils/browser/share.ts").read_text()
expected = 'return "dismissed"'
catch_count = p.count("} catch")
dismissed_count = p.count('return "dismissed";')
print(f"browser share file exists: true")
print(f"file lines: {len(p.splitlines())}")
print(f"catch_count: {catch_count}")
print(f"dismissed_returns: {dismissed_count}")
print(f"bare_catch_block_conflates_implicit_all_rejections: {catch_count == dismissed_count == 1}")
PYRepository: OffCrazyFreak/Disscount
Length of output: 9395
Distinguish dismissal from share failure.
Every navigator.share rejection is reported as "dismissed", including permission or platform failures, so sharing fails fall back to clipboard instead of showing the intended failure toast. Return "dismissed" only for DOMException with name === "AbortError"; return "failed" for other rejections.
Proposed fix
- } catch {
- // Dismissing the sheet is a choice, not a failure to fall back from.
- return "dismissed";
+ } catch (error) {
+ if (error instanceof DOMException && error.name === "AbortError") {
+ return "dismissed";
+ }
+
+ return "failed";
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (navigator.share) { | |
| try { | |
| await navigator.share(data); | |
| return "shared"; | |
| } catch { | |
| // Dismissing the sheet is a choice, not a failure to fall back from. | |
| return "dismissed"; | |
| } | |
| if (navigator.share) { | |
| try { | |
| await navigator.share(data); | |
| return "shared"; | |
| } catch (error) { | |
| if (error instanceof DOMException && error.name === "AbortError") { | |
| return "dismissed"; | |
| } | |
| return "failed"; | |
| } |
🤖 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/utils/browser/share.ts` around lines 16 - 23, Update the
navigator.share error handling in the share flow to return "dismissed" only when
the rejection is a DOMException whose name is "AbortError"; return "failed" for
all other rejection types so callers can fall back to the intended failure
behavior.
Changes: - Delete the flat and glass variants, the switcher, the ?nav= param and the stored preference - Inline the pill classes, taking the scrolled header's bg-background/50 + backdrop-blur-sm + border - Grow the bar to 4.5rem, and add horizontal inset plus inner padding so the active disc clears the pill edges - Grow the active disc and both rings to 3.6rem, and move the USKORO chips to the right with rotate-6 - Move the bottom clearance from <main> to the shell wrapper - Fix the centre cell's active state, which never reflected /products - Derive the cell index from the cells themselves, and release the pointer capture explicitly The three variants existed so the look could be chosen on a device. The pill won, so the rest comes out, and the bar now matches the floating header exactly. Notes: - The footer renders after <main> with mt-auto, so main's padding never protected it: measured, its last 76px sat behind the bar - isActiveIndex short-circuited on !entry.isSearch, so the centre cell could never be active; route match now drives styling for all five cells while isSearch still drives activation - Opening the sheet no longer lights the centre cell, since that is not a route change - The disc is sized against the widest label once bold, which is the state you actually look at: 7.8px around it, 7.2px inside the bar, 11.5px clear of the pill edge at 360px - Dividing the bar's width would skew the cell boundaries now that the pill has inner padding, and a capture outliving its gesture retargeted the next tap to the wrong cell
Changes: - Add a passThroughSurface option to SheetShell - Let the bar opt back into pointer events while the non-modal sheet is open The sheet's surface extends under the bar by design, which meant it swallowed every tap meant for the tabs. Two separate mechanisms had to be beaten, and either fix alone silently fails. Notes: - Radix sets pointer-events inline on the layer, so the class needs the important modifier to win, with children handed them back so the grab handle still drags - Radix also sets pointer-events: none on <body>, which made everything under the sheet unreachable regardless: the bar opts back in, but only while this sheet is open, so it stays inert under real modals - Verified: sheet open, field focused, Pretraži disabled until typed, and all five tabs reachable
Changes: - Set devIndicators: false, since Next's indicator lands bottom-left over Potrošnja - Gate ReactQueryDevtools behind NEXT_PUBLIC_ENABLE_REACT_QUERY_DEVTOOLS - Add the flag to .env.local and .env.local.example, defaulting to false Both overlays sat directly on top of the new bottom bar, covering the first and last tab and making the bar impossible to judge or test. Notes: - Naming follows the existing NEXT_PUBLIC_ENABLE_REACT_SCAN toggle - Both need a dev-server restart: next.config.ts and NEXT_PUBLIC_* are read at compile time - Prettier reformatted next.config.ts, which had drifted out of style
Changes: - Replace the three-variants section with the chosen pill, and why - Record the pointer-events, capture-release and cell-index gotchas - Correct the measured heights, cell widths and touch targets - Note the devtools flag in the automatic-vs-manual table Notes: - The pointer-events entry is the one worth reading: a sheet under fixed chrome needs two independent fixes, and either alone fails silently
Changes: - Rewrite docs/MOBILE-NAV.md from 17 to 19 sections, following the document-subsystem skill - Add "Design decisions and the roads not taken", recording every option considered and why it lost - Add "Config, env vars and flags", covering the viewport flags and the devtools toggle - Replace estimated dimensions with values measured in the browser at 360x740 and 320x568 - Expand the TODO section into board items, hardware-pending items and unscheduled ideas - Add the signed-out behaviour, the compaction mechanics and the SheetShell prop table The previous version was written mid-implementation and then patched twice, so it described three surface variants that no longer exist and guessed at sizes that turned out wrong. It also lost the reasoning behind the choices, which is the part that stops the same questions being reopened. Notes: - Section 12 is the new one that matters: it lists the rejected alternatives, including the seven ideas ruled out as gimmicks, so nobody has to re-derive why swipe-between-tabs or drag-onto-a-tab were dropped - Every dimension in the doc is now a measured value, not an intention: cells are 65.8px at 360px and 57.8px at 320px, not the 72/64 the first draft assumed - Verified before committing: 0 em dashes, no hard wrapping, all 19 anchors resolve, all 28 referenced source paths exist
Changes: - Replace the clamped 120ms feedback ramp with a real gate: no rAF loop and no progress writes until it elapses - Export LONG_PRESS_DEBOUNCE_MS and drop the private FEEDBACK_DELAY_MS - Keep the 450ms fire, so the ring fills over the remaining 250ms The old 120ms threshold was shorter than a deliberate tap, which runs 150-200ms, so every tap on a long-press cell flashed a sliver of ring and the gesture read as broken. Notes: - Product cards share the timer, so they inherit the gate - isPending() stays true through the debounce, which is what still lets a drag abandon the gesture
Changes: - Branch HeaderNavItem on an isLocked prop instead of on item.comingSoon - Compute the lock in HeaderNav with the sidebar's expression - Move the USKORO badge into the shared label, so an admin sees it on a live link The header blocked coming-soon items for admins too, unlike the sidebar, which keeps an escape so a working page behind a badge stays reachable. Notes: - The escape is unreachable in the header today: HeaderNav swaps the whole list for a single dashboard link whenever canAccessDashboard is true, which covers every admin. It is there for consistency and for whenever that layout changes
…e disc Changes: - Cross-fade the centre cell's Search icon into an X while the sheet is open, and toggle the sheet from it - Dismiss the sheet from every other cell, which covers what its pathname effect cannot: re-tapping the tab you are on - Disable USKORO cells for everyone but admins, dropping their tap, focus, scrub tint, disc and long press while keeping the chip - Add use-indicator-opacity, so the active disc dissolves as it slides onto the search cell and resolves as it slides off Three defects found on a phone: the centre cell could open the sheet but never close it, the teaser cells navigated where the header and sidebar refuse to, and on /products the disc's primary/15 sat behind the raised primary circle so green landed on green. Notes: - vaul vetoes every outside dismissal for a non-modal drawer, so the bar is the only thing that can close the sheet, and its open state cannot change mid-gesture - The disc's previous opacity has to live in BottomNav: the element is replaced on every navigation, so a value held inside it would be lost with it - activate() and canLongPress() guard the lock separately, since a disabled button does not stop the list resolving that cell
Changes: - Add subsections on locked teaser cells and on why the active disc fades - Split the long-press timing table into the silent gate and the ring fill, with why 120ms was wrong - Describe the centre cell as a toggle, including vaul's veto on outside dismissal - Add four decision rows to the roads-not-taken table and two new gotchas Notes: - The declined literal 100ms is recorded with its reasoning, so it does not come back
…whole Changes: - Drop passThroughSurface from SheetShell and its only caller, along with the pointer-events-none override - Note on the sheet why z-order alone keeps the tabs tappable The sheet's own padding was pointer-transparent, so a press there hit <html> and vaul's onPress bailed on its contains() check: no drag, no pointer capture. Measured at 360px, that left a 16px dead strip along the sheet's top edge, right where the grab handle sits, so the swipe worked only when a press happened to land on a child element. Notes: - The override was never needed: the bar is z-45 and the sheet z-44, so elementFromPoint resolves all five cells to the nav either way, verified before and after - Verified in the browser at 360x740: no holes left across the sheet's 120px of exposed surface, the swipe closes it, the X closes it, and a tap on another tab still navigates and dismisses it
…sheet Changes: - Replace the two-fix claim with what is actually true: z-order protects the bar, and only the body-level fix is load-bearing - Record the hole-in-the-surface trap, with the measured dead strips and vaul's contains() check - Note that vaul never passes modal down to Radix, which is where the body pointer-events come from - Add the sheet's measured geometry, and the 6.4px handle shadcn leaves behind The previous version prescribed the fix that caused the unreliable swipe.
Changes: - Morph the centre cell's Search icon into ChevronsDown instead of X while the sheet is open - Update the docs and the decision row accordingly Chevrons point the way the sheet actually leaves, which is the same direction as the swipe that dismisses it, where an X reads as "cancel". Notes: - Verified in the browser: chevrons-down at opacity 1 with the search glyph rotated out, and the accessible name still "Zatvori traženje"
…olling Changes: - Add --sheet-bottom-clearance, derived from the bar below md and a plain inset above it - Give DrawerContent an overlayClassName, since it renders the scrim itself - Fix the shell's layer, height cap and bottom inset instead of leaving them to callers - Split the header into sheet-shell-header.tsx, stacking the description under the title - Prevent the default onCloseAutoFocus, matching ModalShell Four sheets had grown four different shapes: only the search sheet cleared the bar, two hand-rolled their bottom padding, and the grab handle sat 6.4px from the content because sr-only on the header container took its padding with it. Notes: - sr-only is position: absolute, so the header row now collapses to its own padding and holds a 16px gap whether or not it draws anything. No conditional. - The visible-description path was previously impossible, which is why the PWA install sheet never migrated off the raw Drawer.
Changes: - Drop the back-to-top FAB from z-50 to z-30 - Drop the PWA install banner from z-50 to z-[41] The sheet scrim now sits at z-43 so sheets can run behind the bar. Anything left at z-50 near the bottom edge would float over that scrim undimmed, and the install banner would cover the very sheet it opens. Notes: - The FAB is desktop-only (hidden md:block), so it never competed with the bar.
Changes:
- Drop product quick actions' hand-rolled safe-area bottom padding
- Rebuild the PWA install instructions sheet on SheetShell with a visible description
Both were carrying geometry the shell now owns, and the install sheet was the last
raw vaul Drawer outside the mobile sidebar.
Notes:
- srOnlyDescription={false} is that sheet's only bespoke prop, and it needed the
header rewrite to render under the title rather than beside it.
… set Changes: - Derive currentValues once, from the values prop when controlled - Compute the toggle payload from it instead of the internal Set The internal Set is seeded at mount and never re-synced, so an owner changing `values` externally desynced it from the display, which read the prop correctly. Clearing the product filters and then picking one chain brought every cleared chain back, because the toggle emitted internal +/- value. Notes: - Predates the search sheet's filters panel; reproduces on the products page's own filters sheet on main.
… button Changes: - Add clear-filters-button.tsx, replacing the local renderClearFilters closure - Add product-filters-trigger.tsx, with an optional expanded chevron for toggles - Rewire product-filters-bar.tsx onto both The search sheet needs the same two controls, and copying them would let the three filter surfaces drift in label, size or disabled state. Notes: - The trigger extends ButtonProps and spreads the rest, so CollapsibleTrigger asChild can drive it as well as a plain onClick.
…roducts Changes: - Add a Filteri toggle under Pretraži that expands the four facet selects inline - Guard it on an exact /products match, in its own component - Add a seedPreferred option so the panel does not race ProductsClient's seeding - Keep the sheet open when navigating to the products list, and drop onSubmitted Expanding grows the sheet upward rather than stacking a second sheet over it, so the query you just typed stays in view and there is one layer to dismiss. The filters have to stay reachable, so arriving at /products no longer closes it. Notes: - The guard is separate because a hook-owning component cannot early-return without changing its hook order between renders. - SearchSheet is mounted in the root layout and useFilterParams writes to usePathname(), so unguarded the panel would rewrite /map?chain=... - Dropping onSubmitted leaves one rule: every exit is a pathname change except the one that should not close it. Scanning still closes, since it opens /products/<ean>.
Changes: - Rewrite section 7 for the fixed geometry, the header split and the full z-index table - Add section 6.1 for the filters panel, plus the /products close-rule table - Add the clearance token to section 10 and five rows to the decisions table - Add four gotchas: sr-only padding, the overlay escape hatch, the layout-mounted sheet's route, and the stale MultiSelect payload - Note the ModalShell responsive-drawer gap and the dead ui/sheet.tsx as follow-ups Verified end-to-end in a browser at 360x740 across all four sheets.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/components/ui/drawer.tsx (1)
55-82: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftKeep project-specific drawer behavior out of the shadcn primitive.
overlayClassNameis custom application behavior added directly tocomponents/ui/drawer.tsx. Move this extension into a custom modal/drawer wrapper, or update the generation source, so regenerating the shadcn primitive cannot discard it.As per coding guidelines: “Do not hand-edit shadcn UI primitives.”
🤖 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/ui/drawer.tsx` around lines 55 - 82, Remove the project-specific overlayClassName extension from DrawerContent in the shadcn Drawer primitive. Move the overlay styling behavior into a custom modal/drawer wrapper, or update the shadcn generation source so regeneration preserves it without hand-editing components/ui/drawer.tsx.Source: Coding guidelines
🤖 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 `@docs/MOBILE-NAV.md`:
- Around line 198-200: Update the CSS code fence in MOBILE-NAV.md around the
utility-class example to specify the css language by changing the opening fence
from an unannotated fence to a css-labeled fence.
In `@frontend/src/app/products/components/product-filters-bar.tsx`:
- Around line 32-42: Update ProductFiltersTrigger in
frontend/src/app/products/components/product-filters-bar.tsx lines 32-42 and
frontend/src/app/products/components/product-search-filters-panel.tsx lines
39-46 to use full width when filters.activeFilterCount is zero, while retaining
the existing flex-1 layout when filters are active; alternatively, pass
alwaysShow to ClearFiltersButton in both sites so the disabled action remains
visible.
In `@frontend/src/components/custom/modal/sheet-shell-header.tsx`:
- Around line 33-40: Update the title/description container in the sheet header
so `srOnlyTitle` no longer applies `sr-only` to the shared column. Apply the
title-only visibility class directly to `DrawerTitle`, while keeping
`srOnlyDescription` applied independently to `DrawerDescription` so the two
props remain composable.
In `@frontend/src/components/custom/modal/sheet-shell.tsx`:
- Around line 79-80: Update the sheet component’s onCloseAutoFocus handling to
restore focus to the control that opened it, either by removing the
preventDefault override so Radix/Vaul can manage focus or by using an explicit
opener ref. Ensure focus returns after Escape and swipe-close instead of being
suppressed.
---
Outside diff comments:
In `@frontend/src/components/ui/drawer.tsx`:
- Around line 55-82: Remove the project-specific overlayClassName extension from
DrawerContent in the shadcn Drawer primitive. Move the overlay styling behavior
into a custom modal/drawer wrapper, or update the shadcn generation source so
regeneration preserves it without hand-editing components/ui/drawer.tsx.
🪄 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: 4b412b07-05ed-474f-b539-25c859b90018
📒 Files selected for processing (32)
docs/MOBILE-NAV.mdfrontend/.env.local.examplefrontend/next.config.tsfrontend/src/app/globals.cssfrontend/src/app/layout.tsxfrontend/src/app/products/components/clear-filters-button.tsxfrontend/src/app/products/components/product-filters-bar.tsxfrontend/src/app/products/components/product-filters-trigger.tsxfrontend/src/app/products/components/product-search-filters-panel.tsxfrontend/src/app/products/components/product-search-filters.tsxfrontend/src/app/products/hooks/use-product-filters.tsfrontend/src/app/products/hooks/use-seed-preferred-filters.tsfrontend/src/app/providers/react-query-provider.tsxfrontend/src/components/custom/bottom-nav/bottom-nav-center-item.tsxfrontend/src/components/custom/bottom-nav/bottom-nav-indicator.tsxfrontend/src/components/custom/bottom-nav/bottom-nav-item.tsxfrontend/src/components/custom/bottom-nav/bottom-nav-ring.tsxfrontend/src/components/custom/bottom-nav/bottom-nav.tsxfrontend/src/components/custom/bottom-nav/use-bottom-nav-pointer.tsfrontend/src/components/custom/bottom-nav/use-indicator-opacity.tsfrontend/src/components/custom/fab/floating-action-button.tsxfrontend/src/components/custom/form/multi-select.tsxfrontend/src/components/custom/header/components/header-nav-item.tsxfrontend/src/components/custom/header/components/header-nav.tsxfrontend/src/components/custom/modal/sheet-shell-header.tsxfrontend/src/components/custom/modal/sheet-shell.tsxfrontend/src/components/custom/product/product-quick-actions.tsxfrontend/src/components/custom/pwa/install-banner.tsxfrontend/src/components/custom/pwa/install-instructions-sheet.tsxfrontend/src/components/custom/search/search-sheet.tsxfrontend/src/components/ui/drawer.tsxfrontend/src/utils/long-press.ts
Comment on lines
+198
to
+200
| ``` | ||
| mx-[0.5rem] px-[0.4rem] rounded-full border bg-background/50 backdrop-blur-sm | ||
| ``` |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Specify the CSS fence language.
markdownlint reports MD040 because this CSS example has no language identifier. Change the opening fence to ```css.
🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 198-198: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 `@docs/MOBILE-NAV.md` around lines 198 - 200, Update the CSS code fence in
MOBILE-NAV.md around the utility-class example to specify the css language by
changing the opening fence from an unannotated fence to a css-labeled fence.
Source: Linters/SAST tools
Comment on lines
+32
to
+42
| <ProductFiltersTrigger | ||
| count={filters.activeFilterCount} | ||
| className="flex-1" | ||
| onClick={() => setIsDrawerOpen(true)} | ||
| > | ||
| <SlidersHorizontal className="size-4" /> | ||
| Filteri | ||
| {filters.activeFilterCount > 0 && ( | ||
| <Badge>{filters.activeFilterCount}</Badge> | ||
| )} | ||
| </Button> | ||
| /> | ||
|
|
||
| {renderClearFilters("flex-1", "default")} | ||
| <ClearFiltersButton | ||
| filters={filters} | ||
| size="default" | ||
| className="flex-1" | ||
| /> |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Keep the filters trigger full width when clearing is unavailable.
ClearFiltersButton returns null with no active filters, while both triggers remain flex-1; the initial mobile row therefore has an empty half-width column.
frontend/src/app/products/components/product-filters-bar.tsx#L32-L42: usew-fullforProductFiltersTriggerwhenactiveFilterCountis zero, or render the disabled clear action withalwaysShow.frontend/src/app/products/components/product-search-filters-panel.tsx#L39-L46: apply the same conditional trigger width or persistent disabled clear action.
Proposed fix
- className="flex-1"
+ className={filters.activeFilterCount > 0 ? "flex-1" : "w-full"}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <ProductFiltersTrigger | |
| count={filters.activeFilterCount} | |
| className="flex-1" | |
| onClick={() => setIsDrawerOpen(true)} | |
| > | |
| <SlidersHorizontal className="size-4" /> | |
| Filteri | |
| {filters.activeFilterCount > 0 && ( | |
| <Badge>{filters.activeFilterCount}</Badge> | |
| )} | |
| </Button> | |
| /> | |
| {renderClearFilters("flex-1", "default")} | |
| <ClearFiltersButton | |
| filters={filters} | |
| size="default" | |
| className="flex-1" | |
| /> | |
| <ProductFiltersTrigger | |
| count={filters.activeFilterCount} | |
| className={filters.activeFilterCount > 0 ? "flex-1" : "w-full"} | |
| onClick={() => setIsDrawerOpen(true)} | |
| /> | |
| <ClearFiltersButton | |
| filters={filters} | |
| size="default" | |
| className="flex-1" | |
| /> |
📍 Affects 2 files
frontend/src/app/products/components/product-filters-bar.tsx#L32-L42(this comment)frontend/src/app/products/components/product-search-filters-panel.tsx#L39-L46
🤖 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/app/products/components/product-filters-bar.tsx` around lines 32
- 42, Update ProductFiltersTrigger in
frontend/src/app/products/components/product-filters-bar.tsx lines 32-42 and
frontend/src/app/products/components/product-search-filters-panel.tsx lines
39-46 to use full width when filters.activeFilterCount is zero, while retaining
the existing flex-1 layout when filters are active; alternatively, pass
alwaysShow to ClearFiltersButton in both sites so the disabled action remains
visible.
Comment on lines
+79
to
+80
| // No trigger to restore focus to, and Radix's body fallback jumps the scroll. | ||
| onCloseAutoFocus={(event) => event.preventDefault()} |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
fd -a 'sheet-shell\.tsx$' . || true
echo "== file outline =="
ast-grep outline frontend/src/components/custom/modal/sheet-shell.tsx --view expanded || true
echo "== relevant lines =="
cat -n frontend/src/components/custom/modal/sheet-shell.tsx | sed -n '1,140p'
echo "== usages of SheetShell =="
rg -n "SheetShell|onCloseAutoFocus|shouldRestoreFocus|restoreFocus|trapFocus" frontend/src -S || trueRepository: OffCrazyFreak/Disscount
Length of output: 7179
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== package declarations =="
for f in package.json frontend/package.json frontend/package-lock.json package-lock.json pnpm-lock.yaml yarn.lock bun.lockb; do
[ -f "$f" ] && echo "--- $f ---" && (rg -n '"vaul"|radix-ui|`@radix-ui/react-dialog`|`@radix-ui/react-slot`|react' "$f" | head -120 || true)
done
echo "== focus prop usage in component family =="
rg -n "initialFocusRef|onOpenAutoFocus|onCloseAutoFocus|closeByClickingOutside|closeByOutsideClick" frontend/src/components/custom/modal frontend/src/components/custom/product/frontend/src/app/products/components/frontend/src/components/custom/search frontend/src/components/custom/pwa -S || true
echo "== invoke sites and refs =="
for f in frontend/src/components/custom/product/product-quick-actions.tsx frontend/src/components/custom/pwa/install-instructions-sheet.tsx frontend/src/components/custom/search/search-sheet.tsx frontend/src/app/products/components/product-filters-bar.tsx; do
echo "--- $f ---"
cat -n "$f" | sed -n '1,130p'
done
echo "== Radix/Vaul focus handling in locked package versions if present =="
if [ -f frontend/package-lock.json ]; then
node - <<'JS'
const fs = require('fs');
const lock = JSON.parse(fs.readFileSync('frontend/package-lock.json','utf8'));
function show(name) {
const key = 'node_modules/'+name;
console.log(key, lock.packages?.[key]?.version || lock.dependencies?.[name]?.version || 'not found');
}
for (const name of ['vaul','`@radix-ui/react-dialog`','`@radix-ui/react-slot`','`@radix-ui/react-use-escape-keydown`','`@radix-ui/react-presence`','`@radix-ui/react-focus-scope`','`@radix-ui/react-focus-guards`']) show(name);
JS
fiRepository: OffCrazyFreak/Disscount
Length of output: 13682
🌐 Web query:
Vaul useDraggableSheet drawer onCloseAutoFocus restore focus trigger ref Radix dialog escape close
💡 Result:
When using Vaul drawers, issues related to focus management (such as the drawer failing to restore focus or trapping focus inappropriately) are often caused by the underlying behavior of the Radix UI Dialog primitive, which Vaul uses as its foundation [1][2]. ### Addressing Focus Restoration and Trapping By default, the Radix Dialog component, and consequently the Vaul Drawer, is designed to act as a modal, which forces a focus trap while open and attempts to restore focus to the trigger element upon closing [1][3]. If you are not using a Drawer.Trigger or if the modal behavior conflicts with your application (e.g., when a drawer is always open or used in non-modal contexts), several common solutions exist: 1. Use onCloseAutoFocus: You can intercept the focus restoration behavior by using the onCloseAutoFocus prop on the Drawer.Content component [4][5]. To prevent the drawer from attempting to restore focus to a missing or incorrect element (which can cause scrolling issues or broken keyboard navigation), you can call e.preventDefault() [4][5]. tsx <Drawer.Content onCloseAutoFocus={(e) => { e.preventDefault(); }} > {/* Content */} </Drawer.Content> 2. Configure the Modal Prop: If your drawer is intended to be non-modal (e.g., it should not trap focus), ensure you are passing modal={false} to the Drawer.Root [4][6]. While older versions of Vaul had issues forwarding this prop to the underlying Radix Dialog, recent updates and patches have been implemented to ensure modal={false} correctly disables the focus trap [6][3]. 3. Manual Focus Restoration: If you need specific focus behavior (such as restoring focus to a specific element that is not the trigger), you can manage this manually using the onOpenAutoFocus and onCloseAutoFocus callbacks [5]. - Capture the active element in onOpenAutoFocus. - Manually call .focus() on that element in onCloseAutoFocus [5]. ### Why These Issues Occur - Browser Behavior: When a dialog/drawer closes, the browser may lose its active focus, causing the page to scroll back to the top or to the trigger element [7][4][8]. This is particularly common in Chrome when the document.activeElement becomes document.body [9]. - Radix Dialog Defaults: Because Vaul is a wrapper around Radix's Dialog primitive, it inherits its accessibility features [1][2]. If the dialog is not configured correctly for your specific use case, these features can interfere with custom focus requirements or lead to trapped focus [10][3]. - Missing Triggers: If you open a drawer programmatically without using a Drawer.Trigger, Radix does not know where to return focus, which can lead to erratic behavior or focus being lost entirely [11][3]. Using a controlled approach with explicit focus management via onCloseAutoFocus is the recommended workaround [5][11].
Citations:
- 1: https://www.radix-ui.com/primitives/docs/components/dialog
- 2: https://emilkowal.ski/ui/building-a-drawer-component
- 3: Focus is trapped inside drawers when always open even with modal=false emilkowalski/vaul#497
- 4: Page is scrolling down when drawer is closed emilkowalski/vaul#569
- 5: [Dialog] Controlling onClose focus on a Controlled Dialog radix-ui/primitives#3319
- 6: fix: Added missing modal prop to Drawer.Root emilkowalski/vaul#580
- 7: Option to disable scrolling back to drawer trigger emilkowalski/vaul#512
- 8: [Dialog] Closing with Escape while focused on conditionally rendered element breaks global key bindings radix-ui/primitives#1546
- 9: [Dialog] Closing controlled dialog does not return focus properly radix-ui/primitives#419
- 10: Drawer does not restore focus to body emilkowalski/vaul#519
- 11: Use of Dialog (controlled) without Dialog.Trigger? radix-ui/primitives#1234
🌐 Web query:
"Vaul" "onCloseAutoFocus" "restoreFocus" "trigger"
💡 Result:
In Vaul (which is built on Radix UI's Dialog primitive), the onCloseAutoFocus prop on the Drawer.Content component is the hook used to control focus restoration when the drawer closes [1][2][3]. By default, when a Vaul drawer closes, it attempts to restore focus to the element that was active before the drawer was opened (typically the trigger) [4][5]. Common behaviors and solutions regarding this trigger include: 1. Preventing unwanted focus restoration: If you want to stop the drawer from automatically returning focus to the trigger (or to prevent the associated page scroll that sometimes occurs during this process), you can use onCloseAutoFocus to prevent the default behavior [6][7][5]. Example implementation: <Drawer.Content onCloseAutoFocus={(event) => { event.preventDefault; }} >... </Drawer.Content> 2. Customizing focus behavior: If you need to restore focus to a specific element other than the original trigger, or perform custom actions upon closing, you can similarly prevent the default and manually call.focus on your desired element [4][5]. Example implementation: <Drawer.Content onCloseAutoFocus={(event) => { event.preventDefault; myCustomRef.current?.focus; }} >... </Drawer.Content> Important Context: - Scrolling issues: Users sometimes report that the page "jumps" or scrolls back to the trigger position upon closing the drawer [6][7]. While preventing the default focus behavior is a common workaround, it effectively stops the library from managing the focus return for you [6][7][5]. - Radix UI Foundation: Because Vaul leverages Radix UI's Dialog primitive, its focus management (including onOpenAutoFocus and onCloseAutoFocus) follows the same architectural patterns [1][2]. If the drawer was opened without a standard trigger or via a custom method, the component may not have a reference to return to, which is when manual implementation becomes necessary [5][8].
Citations:
- 1: https://www.radix-ui.com/primitives/docs/components/dialog
- 2: https://cdn.jsdelivr.net/npm/@adophilus/vaul@0.9.2/src/index.tsx
- 3: hope dialog add complete close event radix-ui/primitives#3128
- 4: [Dialog] Controlling onClose focus on a Controlled Dialog radix-ui/primitives#3319
- 5: [Dialog] Closing controlled dialog does not return focus properly radix-ui/primitives#419
- 6: Option to disable scrolling back to drawer trigger emilkowalski/vaul#512
- 7: Page is scrolling down when drawer is closed emilkowalski/vaul#569
- 8: Use of Dialog (controlled) without Dialog.Trigger? radix-ui/primitives#1234
🌐 Web query:
Vaul issue 580 modal false focus trap onCloseAutoFocus
💡 Result:
The issue regarding Vaul and the modal prop (often discussed in the context of issue #580 and related bugs like #497) stems from the Vaul Drawer.Root component failing to properly forward the modal={false} prop to the underlying Radix UI Dialog.Root [1][2][3]. Because Radix Dialog defaults to modal={true}, Vaul drawers were internally forcing a focus trap even when developers explicitly opted out of modal behavior [1][3]. This often caused conflicts with other focus-management libraries or page-level navigation, occasionally resulting in errors like "Maximum call stack size exceeded" due to competing focus-trap logic [2][3]. Key technical points: - The Fix (#580): A pull request was created to correctly pass the modal prop from Drawer.Root down to the Radix Dialog.Root [1][2]. - Workarounds: Before the fix is fully integrated into a stable release, developers experiencing these issues have used several workarounds, such as explicitly setting autoFocus={true} on the Drawer.Root [1] or using onCloseAutoFocus to prevent default focus behavior: <Drawer.Content onCloseAutoFocus={(e) => { e.preventDefault; }} > [4][5] - Current Status: As of July 2026, while the fix (#580) was merged, users have noted that it may not be present in older versions of the package (such as v1.1.2) [2][3], often necessitating manual patching via tools like pnpm or yarn patch [3]. For those encountering issues with focus trapping or interaction, ensure you are on a version that includes the PR #580 changes, or use the onCloseAutoFocus event handler to manage focus behavior manually until an official update is confirmed for your project environment [3][4].
Citations:
- 1: fix: Added missing modal prop to Drawer.Root emilkowalski/vaul#580
- 2: Request release including #580 (modal prop forwarded to Radix Dialog) emilkowalski/vaul#647
- 3: Focus is trapped inside drawers when always open even with modal=false emilkowalski/vaul#497
- 4: Page is scrolling down when drawer is closed emilkowalski/vaul#569
- 5: Input looses focus emilkowalski/vaul#128
Restore focus when the sheet closes.
onCloseAutoFocus={(event) => event.preventDefault()} disables Radix/Vaul’s focus return, and callers open sheets from real controls rather than Drawer.Trigger. Removing the override or restoring an explicit opener ref keeps keyboard users from losing focus after Escape or swipe-close.
🤖 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/custom/modal/sheet-shell.tsx` around lines 79 - 80,
Update the sheet component’s onCloseAutoFocus handling to restore focus to the
control that opened it, either by removing the preventDefault override so
Radix/Vaul can manage focus or by using an explicit opener ref. Ensure focus
returns after Escape and swipe-close instead of being suppressed.
Changes:
- Default SheetShell to modal={false}, so all four sheets match
- Add a showCloseButton X, used by the quick-actions and PWA install sheets
- Override Radix's inline body lock off a data-sheet-non-modal marker
- Drop the bar's now-redundant pointer-events opt-in
- Give sheets the bar's blur at bg-background/85
- Delete the unused components/ui/sheet.tsx
A sheet here is an addition to the page: you act through it and what is behind
stays available, which is Material's standard bottom sheet, scrimless by
definition. Dialogs are the modal case, and all 12 ModalShell consumers are
self-contained tasks with their own submit.
Notes:
- Non-modal alone did not deliver it. vaul never forwards modal to Radix's
Dialog.Root, so Radix locks <body> inline and vaul's rAF undo loses the race.
Measured after the fix: body reads auto, the page behind scrolls, the sheet
survives an outside click, and a real dialog still reads none.
- vaul also vetoes every outside dismissal when non-modal, so each sheet needs an
explicit way out. The search sheet has the bar's centre cell and the filters
sheet its footer button; the other two gained the X.
- bg-background/85 rather than the bar's /50: at 50% the page ghosts through hard
enough that white inputs look like they float in front of the sheet.
Changes: - Set modal on the product quick-actions sheet, and drop its close X - Replace the Potrošnja cell with Karta, looking ids up across both nav groups - Re-measure the label widths the disc is sized against Quick actions is the one sheet that changes nothing behind it, and two of its three entries open a dialog anyway, so the scrim is honest there and buys back tap-outside-to-close. The other three stay non-modal. Karta and Potrošnja are both comingSoon teasers with a real ComingSoon page, so the swap is a judgement about which a shopper reaches for. Notes: - `map` lives in productNavItems, not userNavItems, so navItem() now searches both. - /spending is in PROTECTED_ROUTE_PREFIXES and /map is not, so the left cell is now reachable signed out. - Widest label is now Praćenje at 42px unbolded and Proizvodi at 44.7px bold, down from Potrošnja's 45.9px, so the 57.6px disc has slightly more room than before.
Changes: - Add useSheetDragUp, an onDragUp prop on SheetShell firing past 40px - Lift the filters' expanded state into SearchSheet so the gesture can open it - Mount the filters panel on every route, always writing to /products - Merge the route guard back into product-search-filters.tsx - Darken all three drawer grab handles to bg-muted-foreground/40 vaul clamps upward movement on a bottom drawer, so the gesture had to be added. Off /products there is no filter state in the URL to amend, so the pick and whatever is typed travel together in one push, which is what the sidebar's filter links already did. Notes: - Plain React pointer props are safe: vaul composes onPointerDown/Move/Up, calling the caller's first, so swipe-to-close is untouched. Verified both still work. - Fires mid-drag, not on release, so the sheet grows under the finger. A 20px nudge does nothing. - The guard no longer needs its own component now that writes are route-safe. - Facets stay empty off /products, so no stray request and no other page's ?q (the map page has one for store names) leaking into product facets. - Verified: 261px collapsed to 559px expanded on both routes, and picking a chain from /watchlist lands on /products?q=kruh&chain=boso with the sheet still open.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/src/components/ui/drawer.tsx (1)
55-117: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHand-edit of a shadcn UI primitive.
This modifies
drawer.tsxdirectly (newoverlayClassNameprop, restyled grab handles) to support the newSheetShellinfrastructure. As per path instructions,frontend/src/components/ui/**/*.{ts,tsx}should not be hand-edited; if future shadcn updates need to be pulled in, consider movingoverlayClassNamehandling into a wrapper inSheetShellinstead, or explicitly documenting this as an intentional, tracked deviation.🤖 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/ui/drawer.tsx` around lines 55 - 117, Remove the hand-edited SheetShell-specific changes from DrawerContent in drawer.tsx, including overlayClassName handling and custom grab-handle styling. Move the overlay customization into the SheetShell wrapper, or explicitly document this intentional deviation from the shadcn primitive update policy if the primitive changes must remain.Source: Path instructions
frontend/src/components/custom/modal/sheet-shell.tsx (1)
68-90: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winWork around vaul’s
modal={false}pointer-events cleanup for controlled drawers.
SheetShelldefaults to non-modal while using controlledopen/onOpenChange, matching vaul issue#492where bodypointer-eventsmay not clear. Since the bottom nav is the intended close target for non-modal sheets, resetdocument.body.style.pointerEventsononOpenChange(false)or apply a targeted global override; don’t drop this inglobals.cssunconditionally, as it would re-enable tapping under real modal sheets.🤖 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/custom/modal/sheet-shell.tsx` around lines 68 - 90, Update SheetShell’s controlled close handling around Drawer’s onOpenChange so that when a non-modal sheet receives open=false, document.body.style.pointerEvents is explicitly reset, while preserving normal modal locking behavior. Use the existing modal prop to scope the workaround and keep the data-sheet-non-modal styling behavior unchanged.
🤖 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.
Outside diff comments:
In `@frontend/src/components/custom/modal/sheet-shell.tsx`:
- Around line 68-90: Update SheetShell’s controlled close handling around
Drawer’s onOpenChange so that when a non-modal sheet receives open=false,
document.body.style.pointerEvents is explicitly reset, while preserving normal
modal locking behavior. Use the existing modal prop to scope the workaround and
keep the data-sheet-non-modal styling behavior unchanged.
In `@frontend/src/components/ui/drawer.tsx`:
- Around line 55-117: Remove the hand-edited SheetShell-specific changes from
DrawerContent in drawer.tsx, including overlayClassName handling and custom
grab-handle styling. Move the overlay customization into the SheetShell wrapper,
or explicitly document this intentional deviation from the shadcn primitive
update policy if the primitive changes must remain.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 902cf4f4-aca1-4604-b169-f93b67ad97ad
⛔ Files ignored due to path filters (1)
nav-karta.pngis excluded by!**/*.png
📒 Files selected for processing (13)
docs/MOBILE-NAV.mdfrontend/src/app/globals.cssfrontend/src/app/products/components/product-search-filters.tsxfrontend/src/components/custom/bottom-nav/bottom-nav-items.tsfrontend/src/components/custom/bottom-nav/bottom-nav.tsxfrontend/src/components/custom/modal/sheet-shell-header.tsxfrontend/src/components/custom/modal/sheet-shell.tsxfrontend/src/components/custom/modal/use-sheet-drag-up.tsfrontend/src/components/custom/product/product-quick-actions.tsxfrontend/src/components/custom/pwa/install-instructions-sheet.tsxfrontend/src/components/custom/search/search-sheet.tsxfrontend/src/components/ui/drawer.tsxfrontend/src/components/ui/sheet.tsx
💤 Files with no reviewable changes (1)
- frontend/src/components/ui/sheet.tsx
Changes: - Add search-nav-button.tsx, the sheet's own nav surface: an outline button with the item's icon, its label and the USKORO badge pinned right - Promote bottom-nav-items.ts's private navItem() lookup to an exported findNavItem() in constants/navigation.ts, so a second surface can resolve items the same way - Point bottom-nav-items.ts at the shared lookup The search sheet had no way into the catalogue except a typed query, so the discounts view was three taps away behind the sidebar. Notes: - The badge sits over the button rather than inside it, as in the sidebar: a disabled button is half-transparent, and a badge that faded with it stopped being readable. - Locked the same way the sidebar, header and bar lock their teasers: comingSoon and not an admin.
Changes: - Delete the products page's own filters sheet; its Filteri button now opens the search sheet with the filters already expanded, via a new openFilters() on SearchSheetContext - Move the expanded-filters state into that context, so every open decides it in one batch: the centre cell always lands compact, Filteri always lands expanded - Split the desktop inline facet row out into product-filters-row.tsx, so the facets hook no longer runs below md where nothing reads it - Make the PWA install sheet modal, alongside product quick actions, leaving the search sheet as the only non-modal one - Drop SheetShell's footer and headerExtra props, which only the deleted sheet used Two sheets were built from the same four facet controls, so a query and the filters narrowing it sat on separate layers. Notes: - initialFocusRef is withheld when the sheet opens expanded, since raising the keyboard would cover the facets you just asked for. - Filters apply live through router.replace, so the deleted sheet's "Prikaži rezultate" button has nothing left to do: the list updates behind the non-modal sheet as you pick. - docs/MOBILE-NAV.md is now stale on §6.1, §7 and §12 and gets one pass before the push.
Changes: - Extract SearchBar's submit button into search-submit-button.tsx, so the inline bars and the sheet share one definition - Render it in the sheet's footer instead of inside SearchBar, below the Popusti shortcut and the filters - Give SearchBar a formId, and the button an owning form attribute, so a button outside the form still submits it and stays its default button - Restore SheetShell's footer prop, which now has a user again The action that runs a search sat above the controls that shape it, and expanding the filters pushed it further from the thumb. Notes: - The footer sits outside the body's scroll container, so the button stays put however tall the filters get. - Enter in the field still submits: an owning form attribute makes an outside button that form's default button. - The footer button is not disabled on an empty query, unlike the inline ones, since that state lives in SearchBar's form. An empty submit browses the whole catalogue, which is a fair action in a sheet that also offers Popusti and Filteri.
Changes: - Add isUnchanged(query) to useSearchNavigation, which owns routeQuery and so already knows what the page is showing - Swap SearchBar's "nothing typed" check for it, covering the landing hero, the sidebar and the products page at once - Mirror the typed value out of SearchBar through a new onQueryChange, so the sheet's footer button can apply the same rule - Add search-sheet-submit.tsx for that button, keeping the useSearchParams call out of the root layout On /products?q=mlijeko the button stayed lit with "mlijeko" still in the field, so the loudest control in the sheet did nothing when pressed. Notes: - Off the search route every query counts as a change, since submitting navigates there, which is why the landing and sidebar bars behave exactly as before. - Deleting the query is now a change too: submitting an empty field over a searched page clears it, where the old rule just greyed the button out. - Filters need no comparison. Each pick writes itself to the URL as you make it, on and off /products, so they are never waiting on a submit. If Pretraži should instead be the apply gate, the picks have to start batching first.
Changes: - Treat an empty field as unchanged in isUnchanged, whatever the URL still holds Clearing the field reset it a render before the navigation landed, so the button compared an empty query against the old one, lit up, then went back out as the URL caught up. Notes: - Nothing is lost by refusing an empty submit: the clear button already drops the query, and instantly. - This also reverts the "submit an empty field to clear the search" side effect from 6497e39, which was never asked for and was the source of the flash.
Changes: - Give RemoveIconButton a tone: destructive keeps the red delete look, neutral outlines it for a reset, and the tooltip follows - Rebuild ClearFiltersButton on it, so the label moves into a tooltip and the button becomes icon-only everywhere - Drop its now-meaningless size prop and let the mobile row shrink it instead of stretching it A ghost button carrying "Očisti filtere" took as much room as the Filteri button beside it, on the screens with the least of it. Notes: - Reusing RemoveIconButton rather than writing a second Tooltip-plus-icon-Button, per AGENTS.md; the two existing red call sites keep today's look through the default tone. - The tooltip cannot open while the button is disabled, since Button sets pointer-events: none there. That only affects the desktop row, which keeps the button mounted to stop the row reflowing, and there is nothing to explain when there is nothing to clear.
Changes: - Always render the button, disabled when there is nothing to clear, so no filter surface reflows as filters come and go - Add showLabel, which the inline row passes: a labelled outline button above md, icon-plus-tooltip below it - Drop alwaysShow, which every surface now gets by default A button that unmounted itself shifted the row every time the last filter went, and the tooltip was carrying a label on the one layout with room to just print it. Notes: - Above md there is no tooltip at all, since the button says what it does. - A disabled trigger cannot open a tooltip anyway, because Button sets pointer-events: none there, so the icon-only variant is silent exactly when it has nothing to explain.
Changes: - Rewrite §6 for the sheet's new contents and its footer submit button, including the form-ownership trick and the isUnchanged rule - Rewrite §6.1 for one filters surface, three ways in, and the collapse-on-open behaviour - Retitle §7's modality section: the split is now search non-modal, the other two modal, with the reasoning per sheet - Drop headerExtra from the prop table, mark the scrim layer as real, and re-do the users table with a modality column - Add §12 rows for the sheet merge, the footer button, the disabled rule, the collapse and the icon-only clear - Re-do §13's file tables, and add §16, §17, §18 and §19 entries for the form attribute, the prerender trap, the clear flash and the batching question The docs described four non-modal sheets and a filters sheet that no longer exists. Notes: - Dropped the measured sheet heights rather than guess new ones: the contents changed and nothing has been re-measured on a device. The clearance figures are token-derived and still hold.
Changes: - Resolve the Praćenje cell's long-press target from the route: on /products/<ean> it opens the watchlist modal for that product, and nowhere else does it do anything - Add use-product-page-ean.ts, which reads the EAN off the path since the bar is in the root layout and never sees the route's params - Fold longPressEnabled into one longPressTarget() helper, so canLongPress and longPress can no longer disagree Notes: - No cache seeding needed, unlike useProductModals: the detail page already fetched the product through the same useGetProductByEan key, so the modal opens populated. - This was recorded as rejected in docs/MOBILE-NAV.md §12, on the grounds that a gesture meaning different things per route cannot be learned. Asked for anyway, so it ships; that row and §8 get rewritten in the doc pass before the next push.
Changes: - Add productPageTarget to IBottomNavItem, a target built from the EAN of the product page you are on - Give Popisi one for add-to-list, keeping its new-list target everywhere else, and move Praćenje's watchlist target onto it - Drop the WATCHLIST_ID special case from bottom-nav.tsx, which the declarative version replaces Both product actions the detail page offers as buttons are now also on the bar, so a one-handed hold reaches either without moving the thumb. Notes: - Praćenje has only a product-page target, so it still has no gesture elsewhere; Popisi has both and swaps between them. - No cache seeding needed for either modal: the detail page already fetched the product through the same useGetProductByEan key. - Two route-dependent cells sharpen the concern recorded in docs §12 about a gesture meaning different things per route. Both targets stay reachable by a visible button on that page, so no action is gesture-only.
Changes: - Give the Karta cell a long-press target of settings/preference, where pinned stores and pinned places are set The map will load pinned stores and centre on the user by itself, so the only part of it worth a shortcut is the preferences behind that behaviour. Notes: - Inert until the map ships, not by longPressEnabled but by the cell's own lock: canLongPress refuses a comingSoon cell for everyone but admins. Admins get it now, everyone gets it the day comingSoon drops. - Four of the five cells now have a hold. Kartice is the only one still wired but disabled, waiting on digital cards.
Changes: - Rewrite §8's gesture table with a column per route context, and add a section on productPageTarget, useProductPageEan and Karta's static target - Record the reversal openly: §8 and §12 both say the per-route gesture was first rejected and why it was overruled - Replace §12's "no long press on Praćenje" and "product cards hold it instead" rows, and add one for Karta - Add §1 rows, a §13 file entry, a §16 note that the contextual targets keep their visible buttons, two §17 rows and a §18 gotcha about useParams being empty in the bar Four of the five cells now have a hold and two of them change meaning by route, none of which the docs described. Notes: - The learnability objection is left standing rather than argued away, since it is the thing to watch if the holds go unused.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@docs/MOBILE-NAV.md`:
- Line 247: Update the Mermaid label for SearchNavButton in the navigation
diagram to indicate that Popusti is disabled only for non-admin users, matching
the SearchSheet admin exception. Keep the existing navigation relationship and
label structure unchanged.
- Line 798: Update the “Inert under dialogs” row in MOBILE-NAV.md to limit the
page hand-back behavior to the non-modal search sheet, while explicitly
distinguishing product quick actions and PWA instruction sheets as modal
overlays that lock the page and cover the bar.
In `@frontend/src/components/custom/common/remove-icon-button.tsx`:
- Around line 28-35: Convert RemoveIconButton to a default export, then update
all imports that reference it—especially the import in
clear-filters-button.tsx—to use the default-import syntax while preserving
existing behavior.
🪄 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: 8e6d341e-0d04-42cd-b852-0576121ad67d
📒 Files selected for processing (19)
docs/MOBILE-NAV.mdfrontend/src/app/products/components/clear-filters-button.tsxfrontend/src/app/products/components/product-filters-bar.tsxfrontend/src/app/products/components/product-filters-row.tsxfrontend/src/components/custom/bottom-nav/bottom-nav-items.tsfrontend/src/components/custom/bottom-nav/bottom-nav.tsxfrontend/src/components/custom/bottom-nav/use-product-page-ean.tsfrontend/src/components/custom/common/remove-icon-button.tsxfrontend/src/components/custom/modal/sheet-shell-header.tsxfrontend/src/components/custom/modal/sheet-shell.tsxfrontend/src/components/custom/pwa/install-instructions-sheet.tsxfrontend/src/components/custom/search/search-bar.tsxfrontend/src/components/custom/search/search-nav-button.tsxfrontend/src/components/custom/search/search-sheet-submit.tsxfrontend/src/components/custom/search/search-sheet.tsxfrontend/src/components/custom/search/search-submit-button.tsxfrontend/src/constants/navigation.tsfrontend/src/context/search-sheet-context.tsxfrontend/src/hooks/use-search-navigation.ts
| openf --> shell | ||
| shell --> focus["initialFocusRef focuses the field<br/>unless opened expanded"] | ||
| shell --> bar["SearchBar<br/>allowScanning<br/>submitButtonLocation=none"] | ||
| shell --> nav2["SearchNavButton<br/>Popusti, disabled"] |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Document the admin exception for Popusti.
SearchSheet locks SearchNavButton only when the item is coming soon and the user is not an admin. This Mermaid label should say it is disabled for non-admins, not unconditionally disabled.
🤖 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 `@docs/MOBILE-NAV.md` at line 247, Update the Mermaid label for SearchNavButton
in the navigation diagram to indicate that Popusti is disabled only for
non-admin users, matching the SearchSheet admin exception. Keep the existing
navigation relationship and label structure unchanged.
| | Icon-only controls | "Očisti filtere" keeps its name in both `aria-label` and a tooltip below `md`, and renders the label outright above it | | ||
| | Focus order | The bar is last in DOM order, so keyboard users reach content first. Its visual position is the bottom anyway | | ||
| | Reduced motion | The disc, the compaction and every scroll are all instant under `prefers-reduced-motion` | | ||
| | Inert under dialogs | Sheets hand the page back, so the bar stays live under them; a real dialog keeps the body locked and its overlay covers the bar anyway | |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Limit the “page hand-back” statement to non-modal sheets.
Only the search sheet is non-modal. Product quick actions and PWA instructions use modal sheets whose overlays lock the page and cover the bar, so this row should distinguish the search sheet from modal sheets.
🤖 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 `@docs/MOBILE-NAV.md` at line 798, Update the “Inert under dialogs” row in
MOBILE-NAV.md to limit the page hand-back behavior to the non-modal search
sheet, while explicitly distinguishing product quick actions and PWA instruction
sheets as modal overlays that lock the page and cover the bar.
Comment on lines
28
to
35
| export function RemoveIconButton({ | ||
| onClick, | ||
| label, | ||
| tone = "destructive", | ||
| loading = false, | ||
| disabled = false, | ||
| className, | ||
| }: IRemoveIconButtonProps) { |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Export this standalone component by default.
Convert RemoveIconButton to a default export and update its imports, including frontend/src/app/products/components/clear-filters-button.tsx.
Proposed fix
- export function RemoveIconButton({
+ export default function RemoveIconButton({- import { RemoveIconButton } from "`@/components/custom/common/remove-icon-button`";
+ import RemoveIconButton from "`@/components/custom/common/remove-icon-button`";As per coding guidelines, “Prefer default exports wherever possible.”
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export function RemoveIconButton({ | |
| onClick, | |
| label, | |
| tone = "destructive", | |
| loading = false, | |
| disabled = false, | |
| className, | |
| }: IRemoveIconButtonProps) { | |
| export default function RemoveIconButton({ | |
| onClick, | |
| label, | |
| tone = "destructive", | |
| loading = false, | |
| disabled = false, | |
| className, | |
| }: IRemoveIconButtonProps) { |
🤖 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/custom/common/remove-icon-button.tsx` around lines 28
- 35, Convert RemoveIconButton to a default export, then update all imports that
reference it—especially the import in clear-filters-button.tsx—to use the
default-import syntax while preserving existing behavior.
Source: Coding guidelines
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.
What this does
Adds an app-style bottom navigation bar below the
mdbreakpoint, so mobile navigation is no longer hamburger-only, and retires the mobile FAB that the bar makes redundant.Full reference doc:
docs/MOBILE-NAV.mdThe two unshipped cells take the outer positions, which are the hardest thumb reach, keeping the layout symmetric with search dead centre.
Why
HeaderNavishidden md:flexandHeaderSearchishidden lg:block, so on a phone every primary destination sat behind the sidebar drawer. NN/g measured that across 179 participants: hidden-only navigation costs over 20% of content discoverability and 15% slower mobile task completion, and nav usage rises from 57% to 86% once a visible surface is added.Their best-performing condition was visible and hidden together, which is why the sidebar stays as overflow rather than being replaced. There is deliberately no "Više" tab.
What's in it
constants/navigation.tsTry it
On a phone or at a mobile viewport:
/productsdown, tap the active tab, then tap it again./shopping-lists/<id>and tick items off, then watch the ring around Popisi.Retiring the mobile FAB
The
FabMenusat atbottom-4 right-4 z-50, exactly where the bar goes, and both of its jobs moved into the bar, so it was redundant with it, not merely colliding:fab-menu.tsx,page-fab.tsxandfab-action.tsare deleted.floating-action-button.tsxandback-to-top-button.tsxstay, the latter nowhidden md:block.Two consequences reviewers should check:
hidden sm:inline-flex. With the FAB gone they were the only tappable path left, and a long press alone would fail WCAG 2.1.1.BackToTopButtonmoved fromsmtomd, or 640-768px would have shown the bar and the FAB together.Things worth reviewing carefully
Cells are
<button>, not<a href>. iOS shows a link-preview popover, a callout and a drag affordance on a long-pressed anchor, and-webkit-touch-callout: noneis unreliable as of iOS 26.1. SEO is unaffected: the prerendered header and sidebar already carry the crawlable links, which commit c0c050c specifically preserved.viewportFit: "cover"was missing. Without it everyenv(safe-area-inset-*)silently resolves to0, with no error, so a bottom bar sits under the iOS home indicator. Also addedinteractiveWidget: "resizes-content"and switched the shell frommin-h-screentomin-h-svh.--spacing: 0.2remmakes every spacing utility 20% small.h-16renders 51px in this project, not 64px. All bar dimensions therefore live in custom properties (--bottom-nav-h,--bottom-nav-total), which also keeps the bar height and the page padding from drifting.Pointer capture retargets the click.
setPointerCaptureon the<ul>means a click no longer lands on the pressed button, so activation runs onpointerupand cells handle only keyboard clicks, discriminated bydetail === 0.Two bottom collisions fixed: the install banner was
fixed inset-x-0 bottom-0 z-50, directly over the bar at a higher z-index, and sonner had no offset so every toast landed on the bar.Refactors folded in
SheetShell,ModalShell's bottom-sheet counterpart. Three sheets had grown three shells; search, product quick actions and product filters now share one, so the grab handle, swipe-to-close and focus behaviour are identical and only the content differs.useProductModals, since the by-ean cache seeding was already duplicated in two components. Those modals are URL-driven and take no props, so without the seed they open empty.scrollWindowTo, becausescrollToTopclaimed to honour reduced motion but did not: browsers apply that to the CSSscroll-behaviorproperty, never to the JSbehavioroption.Known limitations
The iOS keyboard may need one tap. iOS raises it only when
.focus()runs in the same task as the gesture, and a sheet mounting on open is async. The cursor lands in the field; Android and desktop raise the keyboard directly. Avoiding this needs an always-mounted field or a decoy-input hack, both traded away for one consistent sheet shell.No haptics.
navigator.vibratehas never shipped in WebKit and the<input switch>workaround was patched in iOS 26.5. A buzz on Android only is worse than none, so the feel is motion and timing.40% of the bar is teasers. Apple's rule against disabling a tab is clear, but no research endorses teaser destinations in primary nav.
/spendingnow has a value prop and an "Obavijesti me" CTA./digital-cardswas left alone: it is a working page, flagged coming-soon only because barcode rendering is pending.Verification
prettier --checkclean,tsc --noEmitclean,eslintreports 0 errors./products,/shopping-listsand/spendingall return 200.aria-current="page"landing on the right cell.Follow-ups
On the project board in Backlog: the price-drop count badge, the price-drop pulse, enabling the Kartice long press, and selection mode (the bar becoming a contextual action bar with "nađi najjeftiniji lanac za ovih N").
Still deferred: stripping the mobile header to brand plus account, collapsing the mobile footer to legal links, and what happens to
WindowScrollFade, whoseh-28scrim is taller than the bar and washes over the area behind the translucent pill.Second round of changes
Three surface variants were built behind a switcher so the look could be chosen on a real device. That review happened, the pill won, and the other two plus their URL parameter, storage key and dev chip have been removed. Along with it:
bg-background/50 backdrop-blur-sm border, no shadow.isActiveIndexshort-circuited on!entry.isSearch, so the centre cell could never reflect/products; and it lit up merely because the sheet opened. Route match now drives styling for all five cells whileisSearchstill drives activation.<main>, but<Footer>renders after it withmt-auto, so measured, its last 76px sat behind the bar. Moved to the shell wrapper.rotate-6, matching the header nav and landing page convention.devIndicators: false, andReactQueryDevtoolsbehind a newNEXT_PUBLIC_ENABLE_REACT_QUERY_DEVTOOLSflag following the existingNEXT_PUBLIC_ENABLE_REACT_SCANpattern.Two subtle fixes worth a look
The sheet swallowed taps on the bar. It extends under the bar by design, and two independent mechanisms had to be beaten, either of which alone fails silently: Radix sets
pointer-eventsinline on the layer, so the class needs the important modifier; and Radix also setspointer-events: noneon<body>, which made everything underneath unreachable regardless. The bar opts back in only while this non-modal sheet is open, so it stays inert under real modals.Pointer capture outlived its gesture. Relying on the spec's implicit release on
pointeruplet a stale capture retarget the next tap to the wrong cell. Now released explicitly. The cell index is also derived from the cells' own boxes rather than by dividing the bar's width, which the pill's new inner padding would have skewed.🤖 Generated with Claude Code