Skip to content

fix(app): drop the blank band above Scheduled, PRs, and Plugins - #421

Closed
oxwen11 wants to merge 16 commits into
mainfrom
fix/page-header-blank
Closed

oxwen11 wants to merge 16 commits into
mainfrom
fix/page-header-blank

Conversation

@oxwen11

@oxwen11 oxwen11 commented Sep 30, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • On desktop, Scheduled, Pull requests, and Plugins already paint their own top row, so the shell no longer keeps an empty drag strip above it.
  • Traffic lights only cover that top band. The 132px inset is not applied to narrow split controls. Those controls sit below the band, so a 16rem/18rem list keeps its filters and search at full pane width.
  • Scheduled no longer adds pt-8 above its title on any host. Expanded web never had the shell strip; this padding change is the web-visible part.

Verification

  • pnpm exec turbo run typecheck --filter=@getpie/app
  • oxlint on the touched files
  • Web: Scheduled, Pull requests, and Plugins headers sit at the card top.
  • Desktop, sidebar collapsed, pull-request list dragged to its 18rem minimum: before, Authored is clipped by the inset; after, All / Reviewing / Authored stay fully visible and search is not inset.

@pkg-pr-new

pkg-pr-new Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
npx https://pkg.pr.new/oxwen11/pie/@getpie/cli@421

commit: 6fa00f8

@github-actions

github-actions Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

React Doctor skipped this pull request — it changed no React files.

Reviewed by React Doctor for commit 8d0172d.

@oxwen11

oxwen11 commented Sep 30, 2026

Copy link
Copy Markdown
Owner Author

Current-base gate — not merged

  • Context: fix(app): drop the blank band above Scheduled, PRs, and Plugins. Same-repository oxwen11 PR, open, non-draft, targeting main.
  • Head: a71c7e86c0e5ff5b259b4f9529ac05f5aa206fc9.
  • Current base and trusted rules commit: c7c4138afdafaf951349be57cb8acc990a07e0aa. The rule/protection files are unchanged from the previously trusted d7f4b7a95588e5de2aeefbe790e65111e89195f6; the intervening merge is documentation only.
  • Head is behind current main by 1 commit(s). GitHub state: MERGEABLE / BEHIND.

Required CI observed at this head

Additional blocker

Required Check is FAILURE for this head (linked above). No code review or functional verification was started for this newly discovered PR.

Stopped at the current-base gate; not merged. No further code review or independent verification is run against a behind candidate. No branch update, conflict resolution, fix-and-merge or protection bypass was performed. Revisit only after the author supplies a current-base-containing, conflict-free candidate with all required checks present, executed and successful; then restart at CI, review and independent acceptance. Unchanged blockers need no duplicate processing.

@oxwen11

oxwen11 commented Sep 30, 2026

Copy link
Copy Markdown
Owner Author

CI 失败定位与修复方向

  • Head:a71c7e86c0e5ff5b259b4f9529ac05f5aa206fc9
  • 当前 main / 可信规则 commit:8f7546548ca95b7167c2261ebe87f4425806cf65
  • 失败检查日志

pull-request-ui.test.tsx 两项 Chromium 测试失败,页面落入错误边界:useSidebar must be used within a SidebarProvider。优先核对新增 Sidebar 依赖的组件边界,并让测试挂载真实所需的 SidebarProvider;若该组件按公共契约允许在 Provider 外使用,则修正依赖位置,不能仅掩盖异常。修复后跑这两项测试,再跑全套浏览器与 Desktop 顶栏/拖动区域验收。

这是对既有阻塞新增的日志定位,不是代码审查通过。未本地重现或验证修复方案,未修改分支、重跑 CI 或合并。修复/环境恢复并得到满足当前 base 的全绿候选后,重新从 CI → 审查 → 独立验收开始。

@oxwen11

oxwen11 commented Sep 30, 2026

Copy link
Copy Markdown
Owner Author

修复提交与独立审查交接

  • 新 head:7ff74fbcf0c9ef4577b108cbe789e9045edf00f0
  • 集成的 main / 可信规则:8f7546548ca95b7167c2261ebe87f4425806cf65

在真实 Chromium 中复现原失败:4 项测试中 2 项抛 useSidebar must be used within a SidebarProvider。确认生产 AppShell 已提供此上下文,只在测试挂载树补真实 SidebarProvider,未 mock hook、未删断言、未改产品行为。同一 pull-request-ui.test.tsx 修后 4/4 通过;app typecheck 通过;React Doctor 全量扫描 100/100、无诊断;格式/提交钩子通过。同步 main 后重新运行 4 项浏览器测试仍全部通过。

这是实现者的修复自检记录,不是独立验收或批准。已普通推送并触发新 CI;不复用旧 head 的全绿状态。下一步:新 required CI 全部成功后,由独立审查者在干净 worktree 审查完整 PR 并验收。UI 产品路径仍需 Web/Desktop 截图与视频(适用时)。本轮没有合并。

@oxwen11
oxwen11 force-pushed the fix/page-header-blank branch from 7ff74fb to cb72270 Compare September 30, 2026 18:28
@oxwen11

oxwen11 commented Sep 30, 2026

Copy link
Copy Markdown
Owner Author

GitHub-native rebase updated the repaired candidate from 7ff74fbcf0c9ef4577b108cbe789e9045edf00f0 to cb72270069f91a881bf6baef90bdbac1feb5c9a6, on current main/trusted rules a9709fcc6d513f6786dd6c6ce44b13ba8a8e71c3. New CI is pending; no prior-head approval or acceptance is reused. After CI, this implementer-touched PR still requires independent full-PR review and applicable Web/Desktop header verification. No deferred merge armed.

@oxwen11

oxwen11 commented Sep 30, 2026

Copy link
Copy Markdown
Owner Author

Independent review — blocked by a narrow-pane regression

Reviewed head cb72270069f91a881bf6baef90bdbac1feb5c9a6, base/trusted rules a9709fcc6d513f6786dd6c6ce44b13ba8a8e71c3, after all four required checks succeeded.

P2 — collapsed macOS titlebar clearance clips narrow list controls.

  • apps/app/src/features/schedules/schedule-page-list.tsx:37: applying the 132px titlebar inset to the entire Schedule list leaves only 112px usable width at its supported 256px minimum. The unwrapped All/Active/Paused controls cannot fit and overflow-hidden clips them. Search and list rows also unnecessarily inherit the titlebar inset.
  • apps/app/src/features/pull-request/pull-request-page.tsx:147: the equivalent titlebar row cannot fit its labels at the PR list's supported 288px minimum.

Expected: collapse the Desktop sidebar and resize either split list to its minimum; traffic-light clearance must remain while every header/filter control remains visible and usable, with normal search/list width.

This is a source-derived finding independently cross-checked by the coordinator, not a claimed live reproduction. The reviewer’s Chromium geometry probe was blocked before execution by sandbox socket restrictions. A repair needs an executed narrow-layout regression and later actual Desktop/Web acceptance.

Other conclusions: the missing SidebarProvider repair matches production ownership; session titlebar extraction preserves its previous conditions. No new privilege, authorization, persistence or untrusted-data security defect was identified. PR wording claiming expanded Web is entirely unchanged conflicts with the unconditional Schedule pt-8 removal; clarify intended wording rather than treating it as a separately established product defect.

Actual independent checks: app typecheck passed; 27 focused node tests passed. Browser tests were not executed successfully (socket EPERM); no UI/native-drag acceptance or merge approval is claimed. Reviewer worktree remained clean.

The coordinator will handle the straightforward layout correction in an implementation role. After a new head, restart at successful CI, independent review, and real Web/Desktop proof. A fix comment will not resolve this review by itself.

@oxwen11

oxwen11 commented Sep 30, 2026

Copy link
Copy Markdown
Owner Author

auto-merge: no

head cb72270069f91a881bf6baef90bdbac1feb5c9a6 — required CI green (Check, react-doctor, Publish @getpie/cli preview, Continuous Releases, React Doctor), MERGEABLE (BEHIND).

out of .agents/rules/auto-merge-pr.md allowed groups; hits exclusions:

  • functional desktop / windowing behavior: expands titlebar-owner routes, adds useShellTitlebar drag-region + traffic-light inset wiring across Schedules/PRs/Plugins/Packages
  • ui-presentation rejects new hooks, state, handlers, and conditions/control flow (TITLEBAR_ROUTES / ownsTitlebar); not class/style-token-only chrome visibility
  • spacing tweak (pt-8 drop) is inseparable from the shell/drag control-flow change

fail closed; no merge / no update-branch.

@oxwen11

oxwen11 commented Oct 1, 2026

Copy link
Copy Markdown
Owner Author

Queue update after verified #423 merge — waiting at CI

Head 618534c2b9810507541837da9639841d59d1983a now contains main/trusted-rules 0920277492d381b108b520b341c449bcc3e3dbd8 (behind_by=0 verified). GitHub performed a conflict-free merge update guarded by the previous exact head; no conflict resolution or source repair.

Required CI for this new head is queued/in progress at this observation, so no old-head review or acceptance is carried forward. The earlier narrow-window title-bar/layout finding is not resolved by this base sync.

No merge or deferred auto-merge. Continue only after the applicable blocker is resolved and the new version passes required CI → review → independent acceptance.

@oxwen11

oxwen11 commented Oct 5, 2026

Copy link
Copy Markdown
Owner Author

auto-merge: no

head 618534c2b9810507541837da9639841d59d1983a — required CI green (Check, react-doctor, Publish @getpie/cli preview, Continuous Releases, React Doctor), MERGEABLE (BEHIND). Prior no was on cb72270069f91a881bf6baef90bdbac1feb5c9a6; re-reviewed this updated head.

Out of the allowed groups; hits exclusions:

  • functional desktop windowing behavior: new data-drag-region on Packages, Pull requests list/detail/inspect, Scheduled list/filter bar and schedule panel header changes which rows drag the frameless window (packages-page.tsx, pull-request-page.tsx, pull-request-inspect.tsx, schedule-page-list.tsx, schedule-panel.tsx).
  • not ui-presentation-only: new useShellTitlebar hook (shell-chrome.ts), new TITLEBAR_ROUTES + useRouterState route matching and a changed showHeader condition in card-panel.tsx, new dragRegion prop on ScheduleFilterBar.
  • PR body notes desktop after-screenshots were not captured.

iamdin added 2 commits October 5, 2026 12:28
Those pages already paint their own top row. On desktop the empty shell
drag strip sat above it, and Scheduled added pt-8 on top of that. The
page row is the drag strip now.
Traffic lights only cover the top band. Applying that inset to a 16rem
schedule list or an 18rem pull-request list clipped the filters and
squeezed search. Push those controls below the band instead.
@oxwen11
oxwen11 force-pushed the fix/page-header-blank branch from 618534c to 2e818db Compare October 5, 2026 16:44
@oxwen11

oxwen11 commented Oct 5, 2026

Copy link
Copy Markdown
Owner Author

Narrow-pane repair is on this branch, rebased onto current main.

Traffic lights only cover the top band. The 132px inset is no longer applied to the schedule list or the pull-request filter row. Those controls sit below the band, so an 18rem list keeps every label and a normal search width.

Desktop proof, sidebar collapsed, pull-request list dragged to its 18rem minimum:

  • before: Authored is clipped (Au) and the pills start under the toggle
  • after: All / Reviewing / Authored are fully visible; search is not inset

Expanded web has no desktop inset. Scheduled / Pull requests / Plugins headers sit at the card top. Removing Scheduled pt-8 is intentional on every host, not a web-only accident.

desktop-prs-min-collapsed-before

desktop-prs-min-collapsed-after

desktop-scheduled-min-collapsed-after

web-scheduled-after

web-prs-after

web-plugins-after

recording-001.webm
recording-001.webm

@oxwen11

oxwen11 commented Oct 5, 2026

Copy link
Copy Markdown
Owner Author

Blocked: required Check fails on head 5b9472b5

Check run 37348967336 fails in apps/app/src/components/layout/pull-request-ui.test.tsx, in two cases: "reads the selected linked identity…" and "requires a native preview and confirmation…". Both render the "Something went wrong!" error boundary with a useSidebar error.

Cause: this PR adds useShellTitlebar() to PullRequestInspect (pull-request-inspect.tsx), and that hook calls useSidebar(). PullRequestInspect is also rendered outside a SidebarProvider: the test mounts the session content-panel PR view, and any host without the shell sidebar does the same. When that happens, the hook throws.

What's needed: PullRequestInspect must keep working outside a SidebarProvider. For example, only the full-page PR route applies the drag region, or the titlebar hook tolerates a missing provider. Fix the cause rather than wrapping the test in a provider. Then restart from CI.

The navigation rail owns the desktop traffic-light row, so the card no
longer needs a second drag strip. Keep that shell header and drop the
old titlebar inset that would have left a blank band under the rail.
@oxwen11

oxwen11 commented Oct 6, 2026

Copy link
Copy Markdown
Owner Author

Merged current origin/main (3e27258c, navigation rail) into fix/page-header-blank with a merge commit. No force push.

Resolved the shell conflicts by keeping the rail layout:

  • Desktop traffic lights and the sidebar toggle stay on the shell titlebar row above the card. The card no longer paints a second empty drag strip, so Scheduled, Pull requests, and Plugins do not get a blank band above their own headers.
  • Dropped this branch's titlebar inset and clearance row. Those existed to clear lights that used to overlap the card; on the rail they would only add another empty band and squeeze narrow lists.
  • Page headers are the rail's own rows: Scheduled and Pull requests use an h-10 title in the page sidebar; Plugins uses the Customize sidebar. Session title drag stays on the session row, with lights on the rail.

Layout/route browser tests passed: app-rail, app-shell, page-sidebar, shell-columns, packages-page, pull-request-ui (12 tests).

@oxwen11

oxwen11 commented Oct 6, 2026

Copy link
Copy Markdown
Owner Author

auto-merge: no

head 0fe2c6535032. All checks green (Check, react-doctor, Publish @getpie/cli preview, Continuous Releases), MERGEABLE (BEHIND main by 1).

This head has no net diff against main: GitHub compare main...0fe2c6535032 shows 0 changed files across 14 commits. The 02:35Z merge of the navigation rail (#435) kept the rail layout and dropped this branch's titlebar inset and clearance row. So a squash-merge would add nothing, and the blank-band fix now comes from main itself.

Not merging an empty change. I'd suggest closing this PR as superseded by #435, unless a follow-up change is still planned for this branch.

@oxwen11

oxwen11 commented Oct 6, 2026

Copy link
Copy Markdown
Owner Author

Closing as superseded by #435 (merged as 3e27258). Compare main...8d0172d5 shows 0 changed files: resolving the rail conflict dropped this branch's titlebar inset and clearance row, and the blank band above Scheduled / Pull requests / Plugins no longer exists on main because the card does not paint a second drag strip. Nothing left to merge; reopen if a follow-up is planned on this branch.

@oxwen11 oxwen11 closed this Oct 6, 2026
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