Skip to content

fix(ui): pill navbar stacking, gradient fade, and layout - #440

Merged
bordalix merged 2 commits into
masterfrom
wt-pill-fixes-20260313
Mar 18, 2026
Merged

bordalix merged 2 commits into
masterfrom
wt-pill-fixes-20260313

Conversation

@sahilc0

@sahilc0 sahilc0 commented Mar 16, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Portal approach: Pill navbar + gradient haze rendered via createPortal to document.body, escaping ion-app's contain: size layout style that was trapping z-index and fixed positioning
  • Dark bar fix: Override --ion-tab-bar-background: transparent in both :root and .ion-palette-dark to eliminate Ionic's shadow DOM dark bar behind the pill
  • Gradient haze: Radial gradient overlay (pill-navbar-haze) with isolation: isolate stacking context, plus mask-image fade on ion-content::part(scroll) for content fade-out effect
  • Layout spacer: IonTabBar kept in DOM (transparent, position: absolute) so Ionic's flex layout auto-accounts for bottom spacing
  • Tab animations: AnimatedTabIcon with CSS pop animation on tab switch

Known issues / follow-up work

Spacing model is hacky — the current approach uses --pill-navbar-spacer, --pill-navbar-clearance, --pill-navbar-footer-clearance CSS tokens plus per-page padding overrides. This works but is brittle. A follow-up PR will standardize padding/spacing to be less fragile.

Last item in tx history is partially hidden behind navbar — you can still tap on it from behind the pill, so it's not breaking. Will fix in the follow-up spacing PR.

This PR focuses narrowly on fixing the stacking/gradient/dark-bar issues with the new pill navbar. It does not attempt to solve the broader spacing standardization.

Files changed

  • src/App.tsx — Portal PillNavbarOverlay, AnimatedTabIcon, IonTabBar as spacer, has-pill-navbar class
  • src/components/PillNavbarOverlay.tsx — New: portals pill + gradient to document.body
  • src/components/PillNavbar.tsx — Settings tab added, icon animations
  • src/index.css — .pill-navbar-layer, .pill-navbar-haze (radial gradient), dark mode overrides
  • src/ionic.css — --ion-tab-bar-background: transparent, mask-image fade, clearance tokens
  • src/components/ButtonsOnBottom.tsx — .buttons-on-bottom class for CSS targeting
  • src/components/Content.tsx — .content-shell class
  • src/screens/Settings/Delegates.tsx — Per-page pill clearance padding
  • src/icons/Settings.tsx — Default export fix

Test plan

  • Verify gradient haze visible in both light and dark mode
  • Verify no dark bar behind pill navbar in dark mode
  • Verify all 3 tabs (Wallet, Apps, Settings) navigate correctly
  • Verify tab icon pop animation on switch
  • Verify content scrolls and last items are reachable
  • Test on mobile (iOS Safari, Android Chrome) via HTTPS tunnel

Summary by CodeRabbit

  • New Features

    • Persistent bottom tab bar with Wallet, Apps, and Settings, animated tab icons, and an overlay navigation layer for improved tab interactions.
  • Style

    • Updated pill-style navbar visuals, spacing, dark-mode tweaks, reduced-motion safety, and improved safe-area/scroll masking for smooth layout on modern devices.
  • Refactor

    • Tab rendering and navigation wiring simplified for consistent transitions and accessibility.

Portal pill navbar + gradient haze to document.body via createPortal to
escape ion-app's contain:layout. Override --ion-tab-bar-background to
transparent in both light and dark palettes to eliminate the dark bar
behind the pill. Add radial gradient haze overlay and mask-image content
fade. Keep IonTabBar as invisible layout spacer for Ionic's flex system.
@sahilc0
sahilc0 requested review from bordalix and pietro909 March 16, 2026 21:37
@coderabbitai

coderabbitai Bot commented Mar 16, 2026 •

Copy link
Copy Markdown
Contributor

Walkthrough

Adds a persistent bottom IonTabBar with animated tab icons (Wallet, Apps, Settings), a PillNavbarOverlay portal for pill-style navigation, updates PillNavbar to support Settings, moves content styling to class-based shells, and reorganizes CSS/ionic tokens for tab-aware layout, z-index, and reduced-motion handling.

Changes

Cohort / File(s) Summary
Core App & Tab Navigation
src/App.tsx
Adds IonTabs/IonTabBar with AnimatedTabIcon, triggerTabAnim logic, animatingTab state, tab-aware has-pill-navbar class, and moves settings/tab wiring into PillNavbarOverlay. Adjusts OptionsContext usage.
Pill Navbar UI
src/components/PillNavbar.tsx, src/components/PillNavbarOverlay.tsx
Extends PillNavbar to accept onSettingsClick and settingsRef; adds PillNavbarOverlay component that portals a haze layer + PillNavbar into document.body.
Layout & Small Components
src/components/ButtonsOnBottom.tsx, src/components/Content.tsx
Adds buttons-on-bottom CSS class to footer; changes Content to use .content-shell class instead of inline styles.
Icons
src/icons/Settings.tsx
Adjusts SettingsIcon size from 28 to 24.
Styling & Ionic Tokens
src/index.css, src/ionic.css
Reworks pill-navbar overlay layout, centering, z-index and haze gradient; introduces .tab-hidden, .content-shell, pill-navbar spacing tokens, transparent IonTabBar layout, scroll masking, dark-mode overrides, and reduced-motion guards.
Screens
src/screens/Settings/Delegates.tsx
Adds bottom padding to account for pill-navbar clearance.

Sequence Diagram

sequenceDiagram
    actor User
    participant App
    participant PillNavbarOverlay
    participant AnimatedTabIcon
    participant IonTabBar

    User->>PillNavbarOverlay: click tab button (wallet/apps/settings)
    PillNavbarOverlay->>App: onWalletClick/onAppsClick/onSettingsClick
    App->>App: triggerTabAnim(tabName) / set animatingTab
    App->>AnimatedTabIcon: pass animatingTab prop
    AnimatedTabIcon->>AnimatedTabIcon: run icon animation
    AnimatedTabIcon-->>User: icon animates
    App->>IonTabBar: set active tab
    IonTabBar->>App: apply/remove .tab-hidden on non-active pages
    IonTabBar-->>User: active tab content becomes visible
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • Kukks
  • pietro909
  • tiero
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main changes: pill navbar stacking fixes, gradient fade implementation, and layout adjustments are all core changes throughout the PR.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch wt-pill-fixes-20260313
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Mar 16, 2026 •

Copy link
Copy Markdown

Deploying wallet-bitcoin with  Cloudflare Pages  Cloudflare Pages

Latest commit: 78c44f6
Status: ✅  Deploy successful!
Preview URL: https://6ac22164.wallet-bitcoin.pages.dev
Branch Preview URL: https://wt-pill-fixes-20260313.wallet-bitcoin.pages.dev

View logs

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Mar 16, 2026 •

Copy link
Copy Markdown

Deploying wallet-mutinynet with  Cloudflare Pages  Cloudflare Pages

Latest commit: 78c44f6
Status: ✅  Deploy successful!
Preview URL: https://c58fc285.arkade-wallet.pages.dev
Branch Preview URL: https://wt-pill-fixes-20260313.arkade-wallet.pages.dev

View logs

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/index.css (1)

245-264: ⚠️ Potential issue | 🟠 Major

The five-tab bar overflows on narrow phones.

Line 251 caps the bar at 100vw - 24px, but the new five-button layout has a hard minimum of 384px (5 × 72 buttons + 4 × 4 gaps + 8 container padding). On a 320px device that only leaves 296px, so the outer tabs will be pushed off-screen instead of compressing.

📏 Suggested responsive fix
 .pill-navbar {
-  width: fit-content;
+  width: min(460px, calc(100vw - 24px));
   max-width: calc(100vw - 24px);
   display: flex;
   align-items: center;
   gap: 4px;
@@
 .pill-nav-btn {
+  flex: 1 1 0;
-  min-width: 72px;
+  min-width: 0;
   min-height: 44px;
   padding: 6px 12px;
@@
+@media (max-width: 360px) {
+  .pill-nav-btn {
+    padding-inline: 8px;
+  }
+}

Also applies to: 274-283

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/index.css` around lines 245 - 264, The pill bar overflows on narrow
viewports because child tabs have a hard minimum width but the container uses
fit-content; update .pill-navbar and its tab children so the container still
respects viewport space and tabs can shrink: keep the container max-width as
calc(100vw - 24px) (or use max-width: min(100vw - 24px, 100%)) but remove
fit-content sizing (use width: 100% or auto) and ensure each tab element (e.g.,
the tab buttons—selectors like .pill-navbar > button or .pill-button /
.pill-tab) uses flexible sizing such as display:flex; flex: 1 1 0; and
min-width: 0 so five tabs can compress instead of overflowing; optionally add
overflow-x:auto if you prefer scroll on extreme narrow screens.
🧹 Nitpick comments (2)
src/components/PillNavbar.tsx (1)

19-26: Consider simplifying nested ternary with a lookup object.

The three-level nested ternary for ref selection is functional but could be more readable.

♻️ Optional: Use a lookup map
+  const refMap: Record<string, React.RefObject<HTMLDivElement | null>> = {
+    wallet: walletRef,
+    apps: appsRef,
+    settings: settingsRef,
+  }
+
   useEffect(() => {
-    const ref =
-      activeTab === 'wallet'
-        ? walletRef
-        : activeTab === 'apps'
-          ? appsRef
-          : activeTab === 'settings'
-            ? settingsRef
-            : null
+    const ref = refMap[activeTab] ?? null
     if (!ref?.current) return
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/PillNavbar.tsx` around lines 19 - 26, Replace the nested
ternary used to set ref in PillNavbar (the ref variable computed from activeTab)
with a simple lookup map keyed by activeTab (e.g., const refMap = { wallet:
walletRef, apps: appsRef, settings: settingsRef }; const ref = refMap[activeTab]
?? null) so readability is improved and you avoid the three-level nested
ternary; update any references to ref accordingly (walletRef, appsRef,
settingsRef, activeTab).
src/App.tsx (1)

58-79: Animation logic is duplicated with PillNavbar.

Both AnimatedTabIcon here and the useEffect in PillNavbar.tsx use the same class toggle + reflow + animationend pattern. Consider extracting to a shared hook if more instances emerge.

♻️ Optional: Shared hook for animation triggering
// hooks/useAnimationTrigger.ts
import { useRef, useEffect } from 'react'

export function useAnimationTrigger(trigger: boolean, animClass: string) {
  const ref = useRef<HTMLDivElement>(null)

  useEffect(() => {
    if (!trigger || !ref.current) return
    const el = ref.current
    el.classList.remove(animClass)
    void el.offsetWidth
    el.classList.add(animClass)
    const handleEnd = () => el.classList.remove(animClass)
    el.addEventListener('animationend', handleEnd)
    return () => el.removeEventListener('animationend', handleEnd)
  }, [trigger, animClass])

  return ref
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/App.tsx` around lines 58 - 79, AnimatedTabIcon duplicates the animation
toggle/reflow/animationend logic used in PillNavbar; extract that logic into a
shared hook (e.g., useAnimationTrigger) and use it from both components: create
a hook that returns a ref and accepts (trigger: boolean, animClass: string),
move the effect currently inside AnimatedTabIcon (and the similar effect in
PillNavbar) into that hook, then replace the local useRef/useEffect in
AnimatedTabIcon and PillNavbar with the hook and pass animClass (currently
animClass = 'tab-anim-pop') and the animating/trigger prop so both components
reuse the same animation behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@src/index.css`:
- Around line 245-264: The pill bar overflows on narrow viewports because child
tabs have a hard minimum width but the container uses fit-content; update
.pill-navbar and its tab children so the container still respects viewport space
and tabs can shrink: keep the container max-width as calc(100vw - 24px) (or use
max-width: min(100vw - 24px, 100%)) but remove fit-content sizing (use width:
100% or auto) and ensure each tab element (e.g., the tab buttons—selectors like
.pill-navbar > button or .pill-button / .pill-tab) uses flexible sizing such as
display:flex; flex: 1 1 0; and min-width: 0 so five tabs can compress instead of
overflowing; optionally add overflow-x:auto if you prefer scroll on extreme
narrow screens.

---

Nitpick comments:
In `@src/App.tsx`:
- Around line 58-79: AnimatedTabIcon duplicates the animation
toggle/reflow/animationend logic used in PillNavbar; extract that logic into a
shared hook (e.g., useAnimationTrigger) and use it from both components: create
a hook that returns a ref and accepts (trigger: boolean, animClass: string),
move the effect currently inside AnimatedTabIcon (and the similar effect in
PillNavbar) into that hook, then replace the local useRef/useEffect in
AnimatedTabIcon and PillNavbar with the hook and pass animClass (currently
animClass = 'tab-anim-pop') and the animating/trigger prop so both components
reuse the same animation behavior.

In `@src/components/PillNavbar.tsx`:
- Around line 19-26: Replace the nested ternary used to set ref in PillNavbar
(the ref variable computed from activeTab) with a simple lookup map keyed by
activeTab (e.g., const refMap = { wallet: walletRef, apps: appsRef, settings:
settingsRef }; const ref = refMap[activeTab] ?? null) so readability is improved
and you avoid the three-level nested ternary; update any references to ref
accordingly (walletRef, appsRef, settingsRef, activeTab).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f2747891-0a28-4ad5-8cee-6c5f0f43da15

📥 Commits

Reviewing files that changed from the base of the PR and between 40f1f9a and 35229ce.

📒 Files selected for processing (9)
  • src/App.tsx
  • src/components/ButtonsOnBottom.tsx
  • src/components/Content.tsx
  • src/components/PillNavbar.tsx
  • src/components/PillNavbarOverlay.tsx
  • src/icons/Settings.tsx
  • src/index.css
  • src/ionic.css
  • src/screens/Settings/Delegates.tsx

@ghost ghost left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 Arkana Review — wallet#440

Summary: Fixes pill navbar stacking/z-index issues by rendering the navbar via createPortal to document.body, escaping Ionic's contain: size layout style. Adds gradient haze overlay, dark mode fixes, and tab switch animations.

✅ What looks good

  1. Portal approach is the correct fix for escaping Ionic's CSS containment — createPortal to document.body is the standard React pattern for elements that need to break out of layout containment contexts.

  2. Dark mode fix — overriding --ion-tab-bar-background: transparent in both :root and .ion-palette-dark eliminates the shadow DOM dark bar. Good that it targets the right specificity levels.

  3. Author acknowledges tech debt — the spacing model with --pill-navbar-spacer, --pill-navbar-clearance, etc. is explicitly called out as hacky with a follow-up PR planned. Appreciated.

🟡 Observations

  1. Last tx history item partially hidden — acknowledged in the PR. Not blocking but worth tracking.

  2. isolation: isolate on the gradient haze creates a new stacking context. Make sure this doesn't interfere with modals/dialogs that also need to render above the navbar.

🔒 Security

No security concerns. Pure UI/CSS changes — no wallet logic, key handling, or transaction code is touched.

Verdict: Good UI fix, well-scoped. LGTM.

@ghost

ghost commented Mar 18, 2026

Copy link
Copy Markdown

🔍 Review — fix(ui): pill navbar stacking, gradient fade, and layout

Portal escape hatch: createPortal to document.body is the right call here — Ionic's contain: size layout style on ion-app is notorious for trapping fixed-position elements. The isolation: isolate on .pill-navbar-layer creates a clean stacking context.

Observations:

  1. Dead CSS class — .tab-hidden (index.css) is defined but never applied anywhere in this diff or referenced in the TSX. Remove it or wire it up.

  2. Hardcoded colors — #040404, #fbfbfb, #808080 in .pill-nav-btn / .pill-nav-btn--active bypass the existing CSS variable system (--dark50, --ion-color-dark). If the theme palette changes, these won't follow. Consider mapping them to CSS custom properties.

  3. IonTabBar spacer hack — acknowledged in the PR description as brittle. The invisible IonTabBar with opacity: 0 + pointer-events: none + position: absolute is a lot of overrides to keep Ionic's flex layout happy. The CSS tokens (--pill-navbar-spacer, --pill-navbar-clearance, --pill-navbar-footer-clearance) are well-structured, but since everything is absolute-positioned and transparent, a plain div spacer might be simpler and less likely to clash with Ionic's shadow DOM updates.

  4. Mask-image content fade — well done with both -webkit-mask-image and mask-image for cross-browser support. The multi-stop gradient creates a nice perceptual fade.

  5. Reflow trigger — void el.offsetWidth to re-trigger CSS animations is standard. Clean pattern.

  6. Delegates.tsx padding — calc(var(--pill-navbar-clearance) + 24px) as inline padding is functional but per the PR description this will be standardized in a follow-up. Worth tracking.

No security concerns (pure UI). The conditional {tab === Tabs.Wallet && (...)} shorthand change from ternary to && is fine — these components don't return falsy primitives.

Solid incremental fix for the stacking issues. Main action items: remove dead .tab-hidden class, and consider surfacing those hardcoded colors as theme variables.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (2)
src/App.tsx (1)

249-280: Consider simplifying IonTabBar content since it's purely a layout spacer.

Per the PR objectives, the IonTabBar is kept invisible to maintain Ionic's flex layout. However, the buttons currently render full content (icons, text, FlexCol, Focusable, AnimatedTabIcon) that will never be visible. This adds unnecessary DOM nodes and component instances.

♻️ Suggested simplification
 <IonTabBar slot='bottom'>
-  <IonTabButton tab={Tabs.Wallet} onClick={handleWallet} selected={tab === Tabs.Wallet}>
-    <Focusable>
-      <FlexCol centered gap='6px' padding='5px'>
-        <AnimatedTabIcon animating={animatingTab === 'wallet'}>
-          <WalletIcon />
-        </AnimatedTabIcon>
-        Wallet
-      </FlexCol>
-    </Focusable>
-  </IonTabButton>
-  <IonTabButton tab={Tabs.Apps} onClick={handleApps} selected={tab === Tabs.Apps}>
-    <Focusable>
-      <FlexCol centered gap='6px' padding='5px'>
-        <AnimatedTabIcon animating={animatingTab === 'apps'}>
-          <AppsIcon />
-        </AnimatedTabIcon>
-        Apps
-      </FlexCol>
-    </Focusable>
-  </IonTabButton>
-  <IonTabButton tab={Tabs.Settings} onClick={handleSettings} selected={tab === Tabs.Settings}>
-    <Focusable>
-      <FlexCol centered gap='6px' padding='5px'>
-        <AnimatedTabIcon animating={animatingTab === 'settings'}>
-          <SettingsIcon />
-        </AnimatedTabIcon>
-        Settings
-      </FlexCol>
-    </Focusable>
-  </IonTabButton>
+  <IonTabButton tab={Tabs.Wallet} aria-hidden='true' />
+  <IonTabButton tab={Tabs.Apps} aria-hidden='true' />
+  <IonTabButton tab={Tabs.Settings} aria-hidden='true' />
 </IonTabBar>

This removes redundant components and marks the spacer buttons as aria-hidden since PillNavbarOverlay handles the actual navigation.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/App.tsx` around lines 249 - 280, IonTabBar is being used only as an
invisible layout spacer but currently instantiates full tab content
(IonTabButton, Focusable, FlexCol, AnimatedTabIcon, icons, labels) which is
unnecessary; change the IonTabBar children to lightweight spacer buttons by
replacing the full content inside IonTabButton (the ones using
tab={Tabs.Wallet}, tab={Tabs.Apps}, tab={Tabs.Settings}) with minimal
placeholders (e.g., empty or visually-hidden elements), add aria-hidden on those
IonTabButton instances, and keep their onClick handlers (handleWallet,
handleApps, handleSettings) and selected prop behavior intact so
PillNavbarOverlay retains navigation responsibility while reducing DOM/component
overhead.
src/ionic.css (1)

233-256: Consider extracting hardcoded pixel offsets into CSS variables.

The mask gradient uses several hardcoded pixel values (56px, 12px, 18px) that create tight coupling with specific measurements. Per the PR objectives, the spacing model is noted as "hacky and brittle" with a follow-up planned. When that refactor happens, consider extracting these into named tokens for maintainability.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/ionic.css` around lines 233 - 256, The hardcoded pixel offsets in the
mask gradient inside the selector ion-app.has-pill-navbar
ion-content::part(scroll) (56px, 12px, 18px) make spacing brittle; replace those
literal values with new CSS variables (for example --pill-navbar-gradient-top,
--pill-navbar-gradient-mid, --pill-navbar-gradient-bottom or similar semantic
names) and update the gradient calculations to use calc(100% -
var(--pill-navbar-clearance) - var(--pill-navbar-gradient-top)) / calc(100% -
var(--pill-navbar-clearance) - var(--pill-navbar-gradient-mid)) / calc(100% -
var(--pill-navbar-gradient-bottom)) so the spacing tokens are configurable and
self-documented (add default values for the new variables near other CSS
tokens).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/App.tsx`:
- Around line 61-80: AnimatedTabIcon currently always triggers the pop animation
which ignores users' reduced-motion preference; import and call useReducedMotion
inside AnimatedTabIcon (or accept a prefersReduced boolean prop) and bail out of
the effect when reduced motion is requested — e.g., compute const prefersReduced
= useReducedMotion(); update the effect to if (!animating || prefersReduced ||
!ref.current) return; ensure prefersReduced is added to the effect dependencies
so animClass is never added for users who prefer reduced motion.

In `@src/ionic.css`:
- Around line 215-218: The invisible tab buttons (selector ion-tab-bar
ion-tab-button) are still reachable by keyboard/screen-readers; update the CSS
rule to also include visibility: hidden to remove them from focus/layout, and
ensure the component that renders these buttons (PillNavbar) sets tabindex="-1"
on those hidden ion-tab-button elements so they are excluded from keyboard
navigation.

---

Nitpick comments:
In `@src/App.tsx`:
- Around line 249-280: IonTabBar is being used only as an invisible layout
spacer but currently instantiates full tab content (IonTabButton, Focusable,
FlexCol, AnimatedTabIcon, icons, labels) which is unnecessary; change the
IonTabBar children to lightweight spacer buttons by replacing the full content
inside IonTabButton (the ones using tab={Tabs.Wallet}, tab={Tabs.Apps},
tab={Tabs.Settings}) with minimal placeholders (e.g., empty or visually-hidden
elements), add aria-hidden on those IonTabButton instances, and keep their
onClick handlers (handleWallet, handleApps, handleSettings) and selected prop
behavior intact so PillNavbarOverlay retains navigation responsibility while
reducing DOM/component overhead.

In `@src/ionic.css`:
- Around line 233-256: The hardcoded pixel offsets in the mask gradient inside
the selector ion-app.has-pill-navbar ion-content::part(scroll) (56px, 12px,
18px) make spacing brittle; replace those literal values with new CSS variables
(for example --pill-navbar-gradient-top, --pill-navbar-gradient-mid,
--pill-navbar-gradient-bottom or similar semantic names) and update the gradient
calculations to use calc(100% - var(--pill-navbar-clearance) -
var(--pill-navbar-gradient-top)) / calc(100% - var(--pill-navbar-clearance) -
var(--pill-navbar-gradient-mid)) / calc(100% -
var(--pill-navbar-gradient-bottom)) so the spacing tokens are configurable and
self-documented (add default values for the new variables near other CSS
tokens).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6259e534-04f6-490d-8fef-7454cefb9d4b

📥 Commits

Reviewing files that changed from the base of the PR and between 35229ce and 78c44f6.

📒 Files selected for processing (2)
  • src/App.tsx
  • src/ionic.css

Comment thread src/App.tsx
Comment on lines +61 to +80
function AnimatedTabIcon({ children, animating }: { children: React.ReactNode; animating: boolean }) {
const ref = useRef<HTMLDivElement>(null)

useEffect(() => {
if (!animating || !ref.current) return
const el = ref.current
el.classList.remove(animClass)
void el.offsetWidth // Force reflow so removing + re-adding the class triggers the animation
el.classList.add(animClass)
const handleEnd = () => el.classList.remove(animClass)
el.addEventListener('animationend', handleEnd)
return () => el.removeEventListener('animationend', handleEnd)
}, [animating])

return (
<div ref={ref} className='tab-icon-animated'>
{children}
</div>
)
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Tab icon animation should respect reduced motion preference.

AnimatedTabIcon triggers a pop animation but doesn't check useReducedMotion, while page transitions already do (line 189-190). Users who prefer reduced motion won't see page slide animations but will still see tab icon pops, which is inconsistent.

🛡️ Proposed fix to respect reduced motion
-function AnimatedTabIcon({ children, animating }: { children: React.ReactNode; animating: boolean }) {
+function AnimatedTabIcon({ children, animating, reducedMotion }: { children: React.ReactNode; animating: boolean; reducedMotion: boolean }) {
   const ref = useRef<HTMLDivElement>(null)
 
   useEffect(() => {
-    if (!animating || !ref.current) return
+    if (!animating || !ref.current || reducedMotion) return
     const el = ref.current

Then pass prefersReduced to each AnimatedTabIcon instance:

-<AnimatedTabIcon animating={animatingTab === 'wallet'}>
+<AnimatedTabIcon animating={animatingTab === 'wallet'} reducedMotion={prefersReduced}>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/App.tsx` around lines 61 - 80, AnimatedTabIcon currently always triggers
the pop animation which ignores users' reduced-motion preference; import and
call useReducedMotion inside AnimatedTabIcon (or accept a prefersReduced boolean
prop) and bail out of the effect when reduced motion is requested — e.g.,
compute const prefersReduced = useReducedMotion(); update the effect to if
(!animating || prefersReduced || !ref.current) return; ensure prefersReduced is
added to the effect dependencies so animClass is never added for users who
prefer reduced motion.

Comment thread src/ionic.css
Comment on lines +215 to +218
ion-tab-bar ion-tab-button {
opacity: 0;
--background: transparent;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if tabindex is set on IonTabButton elements or if there's other focus prevention
rg -n 'IonTabButton' --type=tsx -A5 | head -60

Repository: arkade-os/wallet

Length of output: 87


🏁 Script executed:

#!/bin/bash
# Explore repository structure to understand file types
ls -la
git ls-files | head -20
echo "---"
# Check what file types exist
find . -type f -name "*.js" -o -name "*.jsx" -o -name "*.ts" -o -name "*.tsx" | grep -v node_modules | head -20

Repository: arkade-os/wallet

Length of output: 3247


🏁 Script executed:

#!/bin/bash
# Look at the ionic.css file around lines 215-218
cat -n src/ionic.css | sed -n '210,225p'

Repository: arkade-os/wallet

Length of output: 436


🏁 Script executed:

#!/bin/bash
# Search for IonTabButton in various file types
rg -n 'IonTabButton' -t js -t jsx -t ts 2>/dev/null | head -40

Repository: arkade-os/wallet

Length of output: 42


🏁 Script executed:

#!/bin/bash
# Check for any tabindex or focus-related attributes in component files
rg -n 'ion-tab-button|tabindex|aria-hidden|visibility.*hidden' src/ | grep -v node_modules | head -50

Repository: arkade-os/wallet

Length of output: 718


🏁 Script executed:

#!/bin/bash
# Look at PillNavbarOverlay to understand the context
cat -n src/components/PillNavbarOverlay.tsx

Repository: arkade-os/wallet

Length of output: 954


🏁 Script executed:

#!/bin/bash
# Check ionic.css for pointer-events on ion-tab-bar
cat -n src/ionic.css | sed -n '200,230p'

Repository: arkade-os/wallet

Length of output: 907


🏁 Script executed:

#!/bin/bash
# Search for ion-tab-bar styling to understand the full context
rg -n 'ion-tab-bar' src/ionic.css -B3 -A3

Repository: arkade-os/wallet

Length of output: 959


Add keyboard focus prevention to hidden tab buttons.

The opacity: 0 hides the buttons visually, but they remain in the DOM and accessibility tree. While pointer-events: none on the parent prevents mouse interaction, it does not prevent keyboard focus. Users navigating with Tab or screen reader users will encounter these invisible buttons, creating a confusing experience.

Add visibility: hidden (to remove from layout while preventing focus) or set tabindex="-1" (to exclude from keyboard navigation) on ion-tab-button to prevent unintended focus since PillNavbar handles the actual interaction.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/ionic.css` around lines 215 - 218, The invisible tab buttons (selector
ion-tab-bar ion-tab-button) are still reachable by keyboard/screen-readers;
update the CSS rule to also include visibility: hidden to remove them from
focus/layout, and ensure the component that renders these buttons (PillNavbar)
sets tabindex="-1" on those hidden ion-tab-button elements so they are excluded
from keyboard navigation.

@bordalix
bordalix merged commit da3a767 into master Mar 18, 2026
5 checks passed
TaprootFreak pushed a commit to TaprootFreak/wallet that referenced this pull request Mar 27, 2026
…e-os#440)

Portal pill navbar + gradient haze to document.body via createPortal to
escape ion-app's contain:layout. Override --ion-tab-bar-background to
transparent in both light and dark palettes to eliminate the dark bar
behind the pill. Add radial gradient haze overlay and mask-image content
fade. Keep IonTabBar as invisible layout spacer for Ionic's flex system.

Co-authored-by: Sahil Chaturvedi <sahilc0@users.noreply.github.com>
Co-authored-by: João Bordalo <bordalix@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants