feat(mobile):Mobile polish and use of frappe ui mobile components. - #284
Open
sadiqxansari wants to merge 5 commits into
Open
feat(mobile):Mobile polish and use of frappe ui mobile components.#284sadiqxansari wants to merge 5 commits into
sadiqxansari wants to merge 5 commits into
Conversation
Contributor
Confidence Score: 4/5The PR is not safe to merge while tall mobile SidePanels remain inaccessible by touch. The attempted scrolling fix applies Files Needing Attention: dashboard/src/components/common/SidePanel.vue Reviews (4): Last reviewed commit: "fix(ui): mobile sizing for list filters,..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
sadiqxansari
force-pushed
the
mobile-polish
branch
2 times, most recently
from
August 13, 2026 11:17
8efe655 to
38494cc
Compare
AppShell hand-rolled one global header for every page, plus an ad-hoc `#header-actions` teleport target that three pages reached into. Both MobileShell and DesktopShell already render a PageHeaderTarget above their scroll region for exactly this: a page declares its own header and it teleports there. We used neither. That bypass is what cost every mobile page its last 48px. Our header sat inside MobileShell's scroll area rather than above it, so the bottom of every page hid behind the nav bar, and the workaround was a hand-managed flex column. Owning the header per page removes the class of bug and turns on what the components already do: a back button, a centred two-line title, tap-the-header to scroll top, tap-the-active-tab to scroll top. The pattern is taken from frappe/gameplan, which is where frappe-ui's shell components were extracted from. Slot names drift between versions — beta.24 takes #left/#right where later releases take #prefix/#suffix — and Vue drops an unknown named slot silently, so these are written against the installed source rather than copied from gameplan's templates. Two things follow from the shell owning the scroll region: Pages hand it their own scroll boxes behind `sm:` prefixes, which is what makes tapping the already-active bottom-nav tab scroll to top. And scroll position is now reset by the router, not by pages remounting. The shell's scroll div outlives the route, so without it a new page opened wherever the last one was left. Only a real path change resets — a query-only replace is the same page and keeps its place. The mobile header's title also doubles as the section switcher: the bottom bar holds four tabs, so Servers, Services, Team and the billing pages were otherwise reachable only by going back to Home first. Home keeps a plain title, since that page already is the list, and takes the mark on the left instead. useBreadcrumbs loses its last consumer here and goes with them. ServerFilters comes along because the servers page now hands it region options grouped by provider, and a prop type has to move with the thing that passes it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The ladder was a four-column table — rail, tier, requirements, price — and the requirements column is the one that carries sentences. At 375px it got whatever the other three left, so "≥ $300.00 paid to date" wrapped to four lines on every rung and the check mark floated away from the text it belonged to. Columns are the wrong primitive when one of them can't be narrowed. Each rung is a wrapping flex row now: name and price share the first line, requirements span the full width beneath, and at `sm` the three fall back into the same columns the table drew. The `<style>` block goes with the table — padding and vertical alignment are utilities once there are no cells. DOM order is the desktop reading order, so the accessibility tree and the tab order match what a sighted desktop user sees; only mobile reorders, and there the sequence read aloud is the same one on screen, wrapped onto two lines. The rail's dot sat 4px below the tier name because its offset assumed a 24px line box. These tokens are 1.15 line-height, not Tailwind's 1.5, so a 14px name centres at ~24px and a 16px one at ~25px. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The Invoices page is a list with a 24rem receipt docked beside it. Nothing docks beside anything at 375px, so the panel stacked under the list at a fixed 384px — and since the page auto-opens the newest invoice, a phone landed on a list with a receipt dangling off the bottom of it. A row pushes /billing/invoices/:name instead, and Back returns to the list. The receipt reads the same on both, so its body moves into InvoiceReceipt and the fetch, the payability rules and the pay wiring move into useInvoiceDetail; the panel and the page supply only their own chrome. Desktop keeps the docked panel, its auto-select and its URLs exactly as they were. Auto-select is desktop-only now: opening a receipt on a phone is a navigation, and landing on the list only to be carried off it is not what tapping Invoices asked for. A `?invoice=` deep link did ask for one by name, so that still goes through — straight to its page. It latches either way, so a later refetch can't act on a query still sitting in the URL and yank you off a list you were reading. Widening past `sm` hands the route back to the list with the same invoice open, the way the settings pages hand back to the dialog. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
SidePanel is a 24rem column that slides in beside the content, hosted as the last child of a `flex h-full` row. Once pages hand their scroll to the shell there is no such row on mobile, so the panel stacked below the content at a fixed 384px on a 375px viewport. Invoices got a route because a receipt is a destination. The rest — wallet history, role capabilities, and the four billing trays — are peeks opened from a card, so they don't want URLs; they want the panel to stop pretending it can dock. Below `sm` it presents as a full-screen sheet, and every consumer gets that from one place. A sheet is modal in a way the docked panel never was: it covers the page rather than sitting beside it. So on mobile it announces as a dialog, takes focus when it opens and gives it back when it closes — otherwise Close leaves focus on a row that is no longer rendered. The Escape guard skips the panel's own root, or the role it just gained would match the "a stacked dialog owns Escape first" test and Escape would stop working entirely. The wallet ledger had its own scroller nested inside the panel's — harmless while the panel was a short column, a trap once it is the whole screen. Same nesting frappe#281 removed from the invoice receipt. RoleCapabilitiesPanel teleported to #team-page-aside, which existed nowhere in the repo: it had never rendered, on any viewport, since the commit that introduced it. Adding the target switches it on — on desktop as well as mobile. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The list toolbar put each filter at a fixed width, so a 160px status filter and a 176px date picker came to 344px against 343px of room and stacked one per row. They grow from a 9rem floor below `sm` instead, which packs two to a row and still lets a third wrap rather than squeezing all three thin. The rest is what a phone reads at arm's length rather than a desk: notification rows and their message text step up a size, the servers list panel stops hanging off the right edge, the region and status filters stack, and the search palette drops its keyboard hints, which are advice a phone can't take. Also: a disabled email field that had gone invisible in dark mode, and the appearance form's row, which was a settings-dialog layout being asked to be a page. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
sadiqxansari
force-pushed
the
mobile-polish
branch
from
August 14, 2026 06:27
38494cc to
59bb8c3
Compare
| :aria-modal="isMobile ? 'true' : undefined" | ||
| :aria-label="isMobile ? title : undefined" | ||
| :tabindex="isMobile ? -1 : undefined" | ||
| class="flex w-[24rem] shrink-0 flex-col border-l border-outline-gray-2 bg-surface-base focus:outline-none max-sm:fixed max-sm:inset-0 max-sm:z-20 max-sm:w-full max-sm:touch-pan-y max-sm:border-l-0" |
Contributor
There was a problem hiding this comment.
When a mobile user pans a SidePanel taller than the viewport, the descendant touch-pan-y cannot override MobileShell’s ancestor touch-none, leaving off-screen wallet, receipt, and capability content unreachable.
sadiqxansari
changed the title
sadiqxansari
changed the title
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.
Adopts frappe-ui's shell pattern — each page declares its own header — and fixes
the mobile surfaces that unlocks.
Patterns are taken from frappe/gameplan,
which is where frappe-ui's shell components were extracted from.
Rebased onto
developafter #285 took frappe-ui from beta.24 to beta.53, whichrenamed
PageHeaderMobile's slots from#left/#rightto#prefix/#suffix.Vue drops an unknown named slot silently, so on the old names every back button
and header action would have rendered nothing at all — worth knowing if this
branch is ever rebased across another bump. Also picked up in that bump:
getScrollContainer()→shellScrollContainer, andonOutsideClickDirective→vOnOutsideClick.Why
AppShellrendered one global header plus an#header-actionsteleport targetthat three pages reached into. Both shells already render a
PageHeaderTargetfor exactly this. Because ours sat inside
MobileShell's scroll area instead,every mobile page lost its bottom 48px behind the nav bar.
What changed
#header-actionsanduseBreadcrumbsare gone. Pages hand their scroll to the shell, so scrollreset moves to the router's
scrollBehavior.holds four tabs, so everything else needed a trip back to Home first.
/billing/invoices/:name);nothing docks 24rem beside a list at 375px. Desktop keeps its panel unchanged.
sm, with dialog semantics and focushandling. Fixes wallet history, role capabilities and the four feat(billing): Overview and Reports - show what the bill is made of #281 trays in
one place.
full-width requirements; desktop keeps its columns.
Verified at 375px and desktop across every in-shell route.
yarn type-checkis clean — the
CollectionActionBanner.vue:80error that was pre-existing ondevelopis fixed by the beta.53 bump.