From cf323b37d5ccb195f71ac5ded3619d3caffcd29c Mon Sep 17 00:00:00 2001 From: omartelo Date: Fri, 30 Oct 2026 17:31:38 -0300 Subject: [PATCH] fix(native): even the Pulls row hover and mirror the screen in its skeleton The tab panels' scroll list reserved the scrollbar's width beside the rows, so a row's hover sat farther from the island's right edge than its left. The bar now overlays the rows, as the index and the sidebar already do. The loading skeleton was a generic stack of bars. It now mirrors the pull request it waits for: the title and stats in the header strip, the tab strip, then the Overview's description beside its facts, or rows shaped like the other tabs' rows. Both rules land in native/DESIGN.md. --- native/DESIGN.md | 8 ++ native/cmd/lich-native/pulls_panels.go | 6 +- native/cmd/lich-native/pulls_skeleton.go | 158 +++++++++++++++++++++++ native/cmd/lich-native/pulls_view.go | 33 ++--- 4 files changed, 181 insertions(+), 24 deletions(-) create mode 100644 native/cmd/lich-native/pulls_skeleton.go diff --git a/native/DESIGN.md b/native/DESIGN.md index 7bf20a318..7030dd108 100644 --- a/native/DESIGN.md +++ b/native/DESIGN.md @@ -132,6 +132,9 @@ island's edge and its hover fill spans the 8px edge to edge. Rows in a list stand `Space(0.5)` to `Space(1.5)` apart; groups of rows `Space(2)`. +A scrolling list lays its bar over the rows (`material.Overlay`), never beside them: a bar that takes its +own width pushes the rows' right edge in, and their hover lands farther from the right edge than the left. + The geometry is shared; the look is not. Every screen lays out on the sidebar's edge, padding and hover shape, and keeps what makes it itself: the Pulls list keeps its seams between rows, the Settings pane its reading column. Matching a screen to the sidebar means its spacing, never its decoration. @@ -155,6 +158,11 @@ edge: the Settings pane sits at `settingsPanePadX` / `settingsPanePadY` (40px / the corner at `settingsCloseInset` (14px). Every other screen body follows the sidebar: its content at 18px from the island's edge, the 8px edge plus a row's 10px (`pullsPadX` in Pulls). +### Loading + +A skeleton mirrors the screen it stands for: the same insets, line heights and columns, a bar where each +piece of text will land, so nothing moves when the content arrives. A generic block of bars is a bug. + ### Boxes inside a screen A thread, a pending comment or a composer is a box on `Sidebar` with `RadiusMD` and `Space(3)` padding. Its diff --git a/native/cmd/lich-native/pulls_panels.go b/native/cmd/lich-native/pulls_panels.go index 1a2785dfa..52354cea9 100644 --- a/native/cmd/lich-native/pulls_panels.go +++ b/native/cmd/lich-native/pulls_panels.go @@ -8,6 +8,7 @@ import ( "gioui.org/layout" "gioui.org/unit" "gioui.org/widget" + "gioui.org/widget/material" "github.com/lichdotdev/lich/native/backend" "github.com/lichdotdev/lich/native/i18n" @@ -248,8 +249,11 @@ func (v *pullView) panelLayout(gtx C, th *ui.Theme, p *pullsScreen, d backend.PR } func (v *pullView) panelList(gtx C, th *ui.Theme, rows []layout.Widget) D { + list := th.ScrollArea(&v.panel) + // The bar overlays the rows so their hover keeps the same edge on both sides. + list.AnchorStrategy = material.Overlay return layout.Inset{Top: ui.Space(1), Bottom: ui.Space(1)}.Layout(gtx, func(gtx C) D { - return th.ScrollArea(&v.panel).Layout(gtx, len(rows), func(gtx C, i int) D { return rows[i](gtx) }) + return list.Layout(gtx, len(rows), func(gtx C, i int) D { return rows[i](gtx) }) }) } diff --git a/native/cmd/lich-native/pulls_skeleton.go b/native/cmd/lich-native/pulls_skeleton.go new file mode 100644 index 000000000..539630e3a --- /dev/null +++ b/native/cmd/lich-native/pulls_skeleton.go @@ -0,0 +1,158 @@ +package main + +import ( + "gioui.org/layout" + "gioui.org/unit" + + "github.com/lichdotdev/lich/native/ui" +) + +// The line heights the skeleton's bars sit in, so each bar takes the room of +// the text it stands for: text-lg, text-sm and text-xs, and the line variant's +// tab list. +var ( + skeletonTitleLine = ui.Space(7) + skeletonSMLine = ui.Space(5) + skeletonXSLine = ui.Space(4) + skeletonTabsLine = ui.Space(9) +) + +// skeletonTabWidths are the tab strip's triggers, Overview to Files. +var skeletonTabWidths = []unit.Dp{ui.Space(18), ui.Space(24), ui.Space(18), ui.Space(14)} + +// detailSkeleton mirrors body while the pull request loads, skeletonHeading +// standing in for its title row: the tab strip and the open tab's content, +// each bar where its text will land. +func (v *pullView) detailSkeleton(th *ui.Theme) layout.Widget { + return func(gtx C) D { + return layout.Flex{Axis: layout.Vertical}.Layout(gtx, + layout.Rigid(func(gtx C) D { + return layout.Inset{Left: pullsToolbarPad, Right: pullsToolbarPad, Top: ui.Space(4)}.Layout(gtx, func(gtx C) D { + return skeletonTabs(gtx, th) + }) + }), + layout.Rigid(th.Separator), + layout.Flexed(1, func(gtx C) D { + if v.tabs.Selected > 0 { + return skeletonRows(gtx, th) + } + return skeletonOverview(gtx, th) + }), + ) + } +} + +// skeletonBar is a bar of width by height centred in a line of height line. +func skeletonBar(th *ui.Theme, width, line, height unit.Dp) layout.Widget { + pad := (line - height) / 2 + return func(gtx C) D { + return layout.Inset{Top: pad, Bottom: pad}.Layout(gtx, th.Skeleton(width, height).Layout) + } +} + +// skeletonHeading is heading's title over the stats line: state, branches +// and author. +func skeletonHeading(gtx C, th *ui.Theme) D { + return layout.Inset{Top: pullsHeadingTop}.Layout(gtx, func(gtx C) D { + return layout.Flex{Axis: layout.Vertical}.Layout(gtx, + layout.Rigid(skeletonBar(th, ui.Space(120), skeletonTitleLine, ui.Space(5))), + ui.VGap(3), + layout.Rigid(func(gtx C) D { + return layout.Flex{}.Layout(gtx, + layout.Rigid(skeletonBar(th, ui.Space(14), skeletonXSLine, ui.Space(3))), + ui.Gap(4), + layout.Rigid(skeletonBar(th, ui.Space(48), skeletonXSLine, ui.Space(3))), + ui.Gap(4), + layout.Rigid(skeletonBar(th, ui.Space(24), skeletonXSLine, ui.Space(3))), + ) + }), + ) + }) +} + +// skeletonTabs is the line tab strip, a bar where each trigger's label sits. +func skeletonTabs(gtx C, th *ui.Theme) D { + var children []layout.FlexChild + for _, w := range skeletonTabWidths { + children = append(children, layout.Rigid(func(gtx C) D { + return layout.Inset{Left: ui.Space(2), Right: ui.Space(3)}.Layout(gtx, skeletonBar(th, w, skeletonTabsLine, ui.Space(3.5))) + })) + } + return layout.Flex{}.Layout(gtx, children...) +} + +// skeletonOverview is overview's description beside its facts, stacked the +// same way on an island too narrow for both. +func skeletonOverview(gtx C, th *ui.Theme) D { + prose := func(gtx C) D { + gtx.Constraints.Max.X = min(gtx.Constraints.Max.X, gtx.Dp(overviewProseWidth+2*pullsPadX)) + return layout.Inset{Left: pullsPadX, Right: pullsPadX, Top: ui.Space(5)}.Layout(gtx, func(gtx C) D { + full := gtx.Metric.PxToDp(gtx.Constraints.Max.X) + children := []layout.FlexChild{layout.Rigid(skeletonBar(th, ui.Space(24), skeletonXSLine, ui.Space(3))), ui.VGap(2)} + for _, w := range []float32{1, 0.94, 0.97, 0.6} { + children = append(children, layout.Rigid(skeletonBar(th, full*unit.Dp(w), skeletonSMLine, ui.Space(3.5)))) + } + return layout.Flex{Axis: layout.Vertical}.Layout(gtx, children...) + }) + } + if gtx.Constraints.Max.X < gtx.Dp(overviewSplitWidth) { + return layout.Flex{Axis: layout.Vertical}.Layout(gtx, + layout.Rigid(func(gtx C) D { + return layout.Inset{Left: pullsPadX, Right: pullsPadX, Top: ui.Space(5)}.Layout(gtx, func(gtx C) D { return skeletonFacts(gtx, th) }) + }), + layout.Rigid(prose), + ) + } + return layout.Flex{}.Layout(gtx, + layout.Flexed(1, prose), + layout.Rigid(func(gtx C) D { return verticalRule(gtx, th) }), + layout.Rigid(func(gtx C) D { + w := gtx.Dp(overviewFactsWidth) + gtx.Constraints.Min.X, gtx.Constraints.Max.X = w, w + return layout.Inset{Left: ui.Space(5), Right: ui.Space(5), Top: ui.Space(5)}.Layout(gtx, func(gtx C) D { return skeletonFacts(gtx, th) }) + }), + ) +} + +// skeletonFacts is facts' sections, a heading over its value each. +func skeletonFacts(gtx C, th *ui.Theme) D { + var children []layout.FlexChild + for i, w := range []unit.Dp{ui.Space(32), ui.Space(40), ui.Space(28)} { + if i > 0 { + children = append(children, ui.VGap(5)) + } + children = append(children, + layout.Rigid(skeletonBar(th, ui.Space(18), skeletonXSLine, ui.Space(3))), + ui.VGap(2), + layout.Rigid(skeletonBar(th, w, skeletonSMLine, ui.Space(3.5))), + ) + } + return layout.Flex{Axis: layout.Vertical}.Layout(gtx, children...) +} + +// skeletonRows is panelList's rows as Conversation, Commits and Checks draw +// them: a glyph beside a line over a dimmer one. +func skeletonRows(gtx C, th *ui.Theme) D { + widths := []float32{0.3, 0.42, 0.25, 0.36, 0.3} + return layout.Inset{Top: ui.Space(1), Bottom: ui.Space(1)}.Layout(gtx, func(gtx C) D { + var rows []layout.FlexChild + for _, w := range widths { + rows = append(rows, layout.Rigid(func(gtx C) D { + return layout.Inset{Left: pullsPadX, Right: pullsPadX, Top: ui.Space(2), Bottom: ui.Space(2)}.Layout(gtx, func(gtx C) D { + full := gtx.Metric.PxToDp(gtx.Constraints.Max.X) + return layout.Flex{Alignment: layout.Middle}.Layout(gtx, + layout.Rigid(th.Skeleton(ui.Space(4), ui.Space(4)).Layout), + ui.Gap(3), + layout.Rigid(func(gtx C) D { + return layout.Flex{Axis: layout.Vertical}.Layout(gtx, + layout.Rigid(skeletonBar(th, full*unit.Dp(w), skeletonSMLine, ui.Space(3.5))), + layout.Rigid(skeletonBar(th, full*unit.Dp(w)*0.8, skeletonXSLine, ui.Space(3))), + ) + }), + ) + }) + })) + } + return layout.Flex{Axis: layout.Vertical}.Layout(gtx, rows...) + }) +} diff --git a/native/cmd/lich-native/pulls_view.go b/native/cmd/lich-native/pulls_view.go index 974d55468..13f13e72c 100644 --- a/native/cmd/lich-native/pulls_view.go +++ b/native/cmd/lich-native/pulls_view.go @@ -161,11 +161,11 @@ func (v *pullView) layout(gtx C, th *ui.Theme, p *pullsScreen, act *pullsAction) read := p.store.detail(p.project.Path, p.selected) switch { case read.val == nil && read.err != "": - return v.withToolbar(gtx, th, p, nil, act, func(gtx C) D { return notice(gtx, th, read.err) }) + return v.withToolbar(gtx, th, p, nil, func(gtx C) D { return notice(gtx, th, read.err) }) case read.val == nil && !read.loaded, read.val == nil && read.loading: - return v.withToolbar(gtx, th, p, nil, act, func(gtx C) D { return detailSkeleton(gtx, th) }) + return v.withToolbar(gtx, th, p, func(gtx C) D { return skeletonHeading(gtx, th) }, v.detailSkeleton(th)) case read.val == nil: - return v.withToolbar(gtx, th, p, nil, act, func(gtx C) D { return notice(gtx, th, i18n.T("pulls.pullsEmptyState.none")) }) + return v.withToolbar(gtx, th, p, nil, func(gtx C) D { return notice(gtx, th, i18n.T("pulls.pullsEmptyState.none")) }) } d := v.edit.shown(*read.val, read, p.store.lastStale()) if stillSettling(d) && !read.loading { @@ -179,14 +179,13 @@ func (v *pullView) layout(gtx C, th *ui.Theme, p *pullsScreen, act *pullsAction) v.edit.openForShot(v.shotEdit, d) v.shotEdit = "" } - return v.withToolbar(gtx, th, p, &d, act, func(gtx C) D { return v.body(gtx, th, p, d, act) }) + heading := func(gtx C) D { return v.titleRow(gtx, th, p, d, act) } + return v.withToolbar(gtx, th, p, heading, func(gtx C) D { return v.body(gtx, th, p, d, act) }) } -// withToolbar lays the strip over w: the crumb, then the pull request's title -// and stats with its actions at the right on the title's line. An island too -// narrow for both on one line puts the actions on their own line above the -// title. -func (v *pullView) withToolbar(gtx C, th *ui.Theme, p *pullsScreen, d *backend.PRDetail, act *pullsAction, w layout.Widget) D { +// withToolbar lays the strip over w: the crumb, then heading, the pull +// request's title row or its skeleton, when there is one. +func (v *pullView) withToolbar(gtx C, th *ui.Theme, p *pullsScreen, heading, w layout.Widget) D { return layout.Flex{Axis: layout.Vertical}.Layout(gtx, layout.Rigid(func(gtx C) D { return layout.UniformInset(pullsToolbarPad).Layout(gtx, func(gtx C) D { @@ -194,10 +193,10 @@ func (v *pullView) withToolbar(gtx C, th *ui.Theme, p *pullsScreen, d *backend.P return layout.Flex{Axis: layout.Vertical}.Layout(gtx, layout.Rigid(func(gtx C) D { return v.crumb(gtx, th, p) }), layout.Rigid(func(gtx C) D { - if d == nil { + if heading == nil { return D{} } - return v.titleRow(gtx, th, p, *d, act) + return heading(gtx) }), ) }) @@ -576,15 +575,3 @@ func (v *pullView) tabStrip(gtx C, th *ui.Theme, p *pullsScreen, d backend.PRDet }), ) } - -func detailSkeleton(gtx C, th *ui.Theme) D { - return layout.Inset{Left: pullsPadX, Right: pullsPadX}.Layout(gtx, func(gtx C) D { - return layout.Flex{Axis: layout.Vertical}.Layout(gtx, - layout.Rigid(th.Skeleton(ui.Space(96), ui.Space(5)).Layout), - ui.VGap(4), - layout.Rigid(th.Skeleton(ui.Space(72), ui.Space(3)).Layout), - ui.VGap(6), - layout.Rigid(th.Skeleton(0, ui.Space(24)).Layout), - ) - }) -}