feat(issues): inline metadata + sticky mobile composer (PP-yxw.9)#1270
Open
timothyfroehlich wants to merge 42 commits intomainfrom
Open
feat(issues): inline metadata + sticky mobile composer (PP-yxw.9)#1270timothyfroehlich wants to merge 42 commits intomainfrom
timothyfroehlich wants to merge 42 commits intomainfrom
Conversation
Design captures the rework from "desktop sidebar + bespoke mobile strip" to a single inline IssueMetadata component (labeled vertical rows with container-query 2-col reflow ≥40rem) plus a mobile-only sticky comment composer. Preserved mockups under docs/superpowers/mockups/ are the historical record of the decisions; design doc is what to build. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Plan converts the design doc into 11 tasks across two PR slices: Phase 1 widens PageHeader.title (3-line type change + tests). Phase 2 introduces IssueMetadata + StickyCommentComposer, restructures page.tsx, deletes IssueSidebar, consolidates OwnerRequirementsCallout placement, and adds the new "Detail Page with Inline Metadata" archetype to the design bible. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…P-yxw.9) Documents the new single-column-with-container-query archetype that issue detail adopts in this rework. Sidebar archetype still applies to machine and location detail until migrated separately. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Code review flagged the new entry as ~8x longer than peer entries in §5, breaking the prescriptive-terse voice. Compressed to ~45 words: one paragraph with class snippet + container-query behavior + Sheet pattern, plus a Note for rationale. Switched container-query notation from 40rem to @XL: (the design-bible §4 canonical breakpoint). Dropped "(or equivalent)" defensive language. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
New labeled-rows component that replaces IssueSidebar (desktop) and the bespoke mobile metadata strip. Five rows (status, priority, severity, frequency, assignee) with container-query 2-column reflow at @XL: container width and an Assignee row that spans both columns in 2-col mode. Each row consumes the existing Update*Form components, preserving permission gating. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Code review identified two test gaps and one dead expression. - Add negative @XL:border-l assertion for Status and Severity rows (left-column rows must NOT have the separator border) - Add smoke test that accessLevel changes rendered output, proving the prop reaches the form components - Drop redundant `?? null` on issue.assignedTo (Drizzle column is string | null; undefined cannot occur) - Drop misleading defensive comment about unused-import linting Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Mobile-only fixed-bottom bar (md:hidden) that opens AddCommentForm in a bottom Sheet. Positioned above the BottomTabBar. Caller is responsible for gating on auth status (signed-in only); the component itself doesn't read auth state. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three review fixes: - Stub AddCommentForm in tests (matches established pattern; avoids jsdom + ProseMirror fragility per issue-detail-permissions test) - Add min-h-[44px] to trigger button to meet Apple HIG / Material touch-target minimums (consistent with IssueMetadata rows) - Align opacity to bg-card/90 matching BottomTabBar (visual consistency between adjacent nav chrome) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The callout was hidden below @XL: container width and re-rendered in the page chrome on mobile, producing a dual-render. With the new inline-metadata layout, the callout has a single canonical location inside the timeline at every viewport. Margin alignment via @XL:ml-20 is preserved. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…y composer (PP-yxw.9) Removes the desktop sidebar grid (md:grid-cols-[1fr_320px]), the mobile-only metadata strip (border-y py-2), and the dual breadcrumb implementations. Replaces them with PageContainer + eyebrow row + widened PageHeader (using EditableIssueTitle as the title node) + subtitle row + IssueMetadata + IssueTimeline. Mobile signed-in users get a sticky bottom composer. OwnerRequirementsCallout is no longer rendered here — it lives inside IssueTimeline at every viewport now. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three code-review fixes layered on the page restructure: - Bump sticky-composer clearance from 52px to 64px in PageContainer className override; the actual composer renders at ~61px (border + py-2 + 44pt button + py-2). Adds 3px breathing room on notch-free devices where env(safe-area-inset-bottom) = 0. - Add data-testid="issue-detail-subtitle" to the subtitle div so Task 9's reporter-variations E2E migration has a stable selector (replacing the deleted "issue-sidebar" testid). - Update design-bible §9 typography entry: detail pages now use text-3xl font-bold at every viewport (was text-xl font-extrabold on mobile under the old archetype). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ata (PP-yxw.9) IssueSidebar is removed; its functionality now lives in IssueMetadata (5 editable fields) and the page subtitle row (reporter, date, watchers, watch button). E2E and unit test selectors updated: - e2e/smoke/issues-crud.spec.ts: drop mobile/desktop divergence assertions; use unified issue-metadata-grid + BackToIssuesLink - e2e/full/reporter-variations.spec.ts: migrate 7 tests from issue-sidebar to issue-detail-subtitle - src/test/unit/components/issues/issue-detail-permissions.test.tsx: swap IssueSidebar import for IssueMetadata; update final test to verify issue-metadata-grid renders (WatchButton now lives in page) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The migrated "hides the watch button" test had two issues caught in
code review: the WatchButton/SidebarActions mocks intercept modules
IssueMetadata never imports (dead), and the assertion that
queryByTestId("mock-watch-button") returns null is tautological —
it would pass even with the page's permission gate removed.
Reduced to a smoke check that IssueMetadata renders the metadata
grid for unauthenticated users. WatchButton permission gating is
covered at the page level by e2e/smoke/issue-detail-permissions.spec.ts.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three scenarios for the new mobile-only sticky comment composer: - Mobile signed-in: sticky bar visible; tap opens the comment Sheet - Mobile signed-out: sticky bar hidden; inline "Log in to comment" placeholder in IssueTimeline shows instead - Desktop signed-in: sticky bar never renders (md:hidden); inline composer at end of timeline is the only one Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two corrections discovered while executing Phase 2: - Task 8 PageContainer override: bumped 52→64px sticky-clearance after measuring actual composer height (61px); added explanatory note. - Task 9 test paths: the design doc's deliverables table guessed e2e/full/issues-crud.spec.ts but the stale assertions actually live in e2e/smoke/issues-crud.spec.ts AND e2e/full/reporter-variations.spec.ts. Updated Task 9 with both file paths and migration guidance for each. These match the implementation that landed in dffbc29 and 72777ca. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Updates to Preview Branch (feat/issue-detail-rework-PP-yxw.9) ↗︎
Tasks are run on every commit but only new migration files are pushed.
View logs for this Workflow Run ↗︎. |
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors the issue detail page layout to remove the desktop sidebar/mobile strip divergence by introducing a single inline IssueMetadata component (container-query reflow) and adding a mobile-only sticky comment composer that opens AddCommentForm in a Sheet.
Changes:
- Add
IssueMetadataand update issue detail page structure to use eyebrow →PageHeader→ subtitle → metadata → timeline (deleteIssueSidebar). - Add
StickyCommentComposer(mobile-only, auth-gated) plus unit/E2E coverage around visibility + sheet open behavior. - Update E2E selectors/tests and extend design documentation + preserved mockups for the new archetype.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/unit/components/issues/issue-detail-permissions.test.tsx | Updates unit test to assert IssueMetadata renders for unauthenticated users. |
| src/components/issues/StickyCommentComposer.tsx | New mobile-only fixed-bottom trigger that opens a comment sheet. |
| src/components/issues/StickyCommentComposer.test.tsx | Unit tests for sticky composer rendering + sheet open behavior. |
| src/components/issues/IssueTimeline.tsx | Makes OwnerRequirementsCallout render at all viewports (removes hidden). |
| src/components/issues/IssueSidebar.tsx | Deletes the legacy sidebar component (replaced by inline metadata + subtitle). |
| src/components/issues/IssueMetadata.tsx | New inline metadata grid with container-query 1→2 column reflow. |
| src/components/issues/IssueMetadata.test.tsx | Unit tests for grid structure/container-query classes and prop forwarding. |
| src/app/(app)/m/[initials]/i/[issueNumber]/page.tsx | Restructures issue detail page to new inline-metadata flow; adds sticky composer and new subtitle. |
| e2e/smoke/issues-crud.spec.ts | Updates smoke assertions to the unified layout (issue-metadata-grid, always-visible back link). |
| e2e/full/reporter-variations.spec.ts | Migrates reporter assertions from sidebar to the new subtitle row. |
| e2e/full/issue-detail-sticky-composer.spec.ts | Adds focused E2E coverage for sticky composer visibility rules across auth + viewport. |
| docs/superpowers/specs/2026-05-01-issue-detail-rework-design.md | Adds the full design spec documenting the new issue detail archetype and behavior. |
| docs/superpowers/mockups/2026-05-01-issue-detail-rework/pill-organization.html | Preserved brainstorming mockup artifact. |
| docs/superpowers/mockups/2026-05-01-issue-detail-rework/mobile-metadata-strategy.html | Preserved brainstorming mockup artifact. |
| docs/superpowers/mockups/2026-05-01-issue-detail-rework/labeled-directions.html | Preserved brainstorming mockup artifact. |
| docs/superpowers/mockups/2026-05-01-issue-detail-rework/issue-detail-competitive.html | Preserved brainstorming mockup artifact. |
| docs/superpowers/mockups/2026-05-01-issue-detail-rework/header-status-placement.html | Preserved brainstorming mockup artifact. |
| docs/superpowers/mockups/2026-05-01-issue-detail-rework/d2-final.html | Preserved brainstorming mockup artifact. |
| docs/superpowers/mockups/2026-05-01-issue-detail-rework/authentic-issue-detail.html | Preserved brainstorming mockup artifact. |
| docs/superpowers/mockups/2026-05-01-issue-detail-rework/README.md | Documents the mockup decision arc and how to verify implementation against it. |
| .agent/skills/pinpoint-design-bible/SKILL.md | Updates design bible archetypes to add “Detail Page with Inline Metadata” for issue detail. |
The Task 8 page restructure dropped the <h2 className="hidden md:block"> Activity heading that the issues-crud smoke test relies on (toHaveCount(isMobile ? 0 : 1) at line 221). Restored it inside the section wrapping IssueTimeline. Mobile remains heading-less per the existing assertion; desktop shows the section label as before. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The full E2E spec was looking for issue-status-badge testid on the detail page after logging in as admin. That testid is rendered by UpdateIssueStatusForm only in the unauthenticated fallback path (IssueBadge component, line 110 of update-issue-status-form.tsx). Authenticated users see an interactive StatusSelect instead — testid issue-status-select — which renders the current status label as its visible value. Switched the admin-path test assertion to issue-status-select + toContainText(/New/i). The unauthenticated smoke test (issue-detail-permissions.spec.ts) still asserts on the badge testid for the read-only path; that path is unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Radix Select Content uses Floating UI to compute dropdown positioning and available space. When the SelectTrigger sits inside an overflow-hidden ancestor combined with a CSS containment context (@container's container-type: inline-size), the popper calculation can collapse and the SelectContent fails to mount. This breaks status-overhaul.spec.ts: the trigger click registers (trigger receives focus) but no SelectContent appears in the DOM, so the option testid is never found and click() times out. The same Radix Select on /report works in the same test because it has neither container-type nor overflow-hidden ancestors. Removing overflow-hidden from the grid wrapper restores popper behavior. Visual cost: rounded corners on the IssueMetadata grid no longer clip inner row borders. The outer rounded border still renders correctly.
Owner
Author
|
- Add focus-visible rings to interactive timestamp triggers and StickyCommentComposer button. - Stabilize RelativeTime first-paint to avoid SSR/CSR mismatch. Addresses Copilot a11y/perf threads on #1270.
timothyfroehlich
added a commit
that referenced
this pull request
May 5, 2026
- pr-watch.py now paginates the reviewThreads GraphQL query via pageInfo.hasNextPage cursor, so PRs with >100 review threads are audited completely. Previously the readiness audit only inspected the first 100 threads, silently missing unresolved Copilot feedback on large PRs (e.g. #1270 has 33 active threads, more accumulate over time). - Fixes ruff F541 (extraneous f-prefix) errors that broke Fast Linters on the prior commit. Addresses GitHub Copilot review comments on PR #1287.
4 tasks
timothyfroehlich
added a commit
that referenced
this pull request
May 6, 2026
…00s) (#1281) * docs(agents): edit canonical specs in place, no divergence notes (PP-00s) Adds a "Keeping specs aligned" subsection to AGENTS.md section 5 instructing agents to edit canonical spec docs in place rather than appending divergence notes at the bottom of files. Codifies the lesson from PR #1270 review churn where spec/implementation drift produced repeated Copilot back-and-forth. Also tells agents that finding an existing divergence note is a signal to fold the content back into the canonical text, not preserve the annotation. No automation — process guidance only. Single source of truth = the spec docs themselves. Refs PP-2on (epic). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(agents): scope spec-alignment to canonical, drop PR-specific rationale (PP-00s) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs(agents): fix archetypes reference to point to SKILL.md sections (PP-00s) The previous text referenced a non-existent .agent/skills/pinpoint-design-bible/archetypes/ directory. Page and Modal Archetypes are sections inside SKILL.md (§5 and §17). --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…-yxw.9)
Replace `<span tabIndex={0}>` with `<button type="button">` inside the
TooltipTrigger so the tooltip can be reached via keyboard focus, not
just hover. Browser button defaults are reset (`bg-transparent p-0
text-left [font:inherit]`) to preserve the original inline visual.
Resolves Copilot thread on page.tsx (TooltipTrigger asChild + non-focusable
span).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…yxw.9) Previously, if `value` changed to an invalid date, the effect early-returned without resetting `label`, so a stale relative-time string would keep rendering even though the prop now represents a different (or invalid) instant. Now `setLabel(null)` runs in the invalid branch so the resolved fallback wins. Resolves Copilot thread on RelativeTime.tsx:25. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…P-yxw.9)
The strict-mode "2 Unwatch buttons" violation was caused by the previous
desktop-sidebar + mobile-metadata-strip dual placement. The Wave 2/3A
inline-metadata redesign renders exactly one WatchButton in the page
header, so getByRole("button", { name: /Unwatch Issue/i }) is no longer
ambiguous.
Removes 2 test.fixme calls in issues-crud-extended.spec.ts (lines 44, 70).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
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.
Summary
Slice 2 of the issue detail page rework. Replaces the divergent desktop sidebar / bespoke mobile composer pattern with a unified inline-metadata + sticky-composer layout that scales from mobile to desktop via container queries. Touches 30+ files (+6405 / -365), mostly in
src/components/issues/,src/app/(app)/m/[initials]/i/[issueNumber]/, and supporting design docs.Status (2026-05-06)
This PR is not yet ready for merge. It's gated on the binding rule established by skip-PR #1290:
Wave 3A is now complete on this PR. The 2 fixmes that #1270 owned (PP-49m) have been removed. The other 8 fixmes on main are tracked by #1291, #1293, and a future PP-q9r iter 3 PR — #1270 cannot merge until those land.
Wave 3A — what landed
The 17 unresolved Copilot threads collapsed into 6 distinct code fixes plus a fixme removal. All shipped in commits below.
Priority 1 — privacy regression (FIXED)
allUsersexposed to anon/guest viewers. Full member roster used to serialize into the RSC payload for unauthenticated and guest viewers even though they couldn't use the assignee picker. NowallUsersfetch is gated onissues.update.triage; non-triage viewers receive only the currently-assigned user so the readonly assignee display still resolves a name.src/app/(app)/m/[initials]/i/[issueNumber]/page.tsxsrc/components/issues/IssueMetadata.tsxe25cac27,3256aeb1Priority 2 — mobile UX regression (FIXED)
Double comment composer on mobile. Sticky composer + inline
AddCommentFormwere both rendering for authenticated mobile users. Inline composer is now hidden belowmd:when authenticated; the unauthenticated "Log in to comment" placeholder remains visible at all viewports (no sticky for guests).src/components/issues/IssueTimeline.tsxe224a77bPriority 3 — spec/code alignment (FIXED)
@xl:(576px) vs design doc@container (min-width: 40rem)— design doc updated to use Tailwind shorthand notation matching impl.OwnerRequirementsCalloutplacement — decision: page-level aboveIssueMetadatais canonical. Spec already documents this (D6 in2026-05-01-issue-detail-rework-design.md); PR description has been corrected.max-w-3xl(narrow) vs Design Bible'smax-w-6xl— Design Bible updated to reflect the narrow archetype.docs/superpowers/specs/2026-05-01-issue-detail-rework-design.md,.agent/skills/pinpoint-design-bible/SKILL.mde224a77b,8d1ad185Priority 4 — small nits (FIXED)
IssueMetadataborder-bottom in 2-col mode — addedlastInColumnAtXlflag on Severity row so the bottom edge is symmetric. Commit:e224a77b/1c969f08.<span tabIndex={0}>with<button type="button">insideIssueUpdatedTimestampso the tooltip is keyboard-reachable. Commit:2a5ab3f2.RelativeTimestale label whenvaluebecomes invalid — effect now resetslabelin the invalid branch. Commit:ff7568bc.PP-49m fixme removal (FIXED)
The strict-mode "2 Unwatch buttons" violation that those fixmes guarded was caused by the prior dual-placement layout. The redesign renders exactly one
WatchButtonin the page header, so the selector is unambiguous. Commit:9e2471c7.Deferred to follow-up beads
RelativeTimeshared ticker (PP-nut, P3)Each
<RelativeTime>instance creates its own 60ssetInterval. On long issue timelines that's N timers fanning out. Implementing a shared ticker context (provider +useSyncExternalStorehook + rewrite ofRelativeTime) is a real ~100 LOC refactor across 3-4 files, not a one-line nit. Spun out so #1270 stays surgical. Visual output is unchanged either way.Already addressed (earlier waves)
f06b7aa8): focus-visible rings on tooltip triggers +StickyCommentComposer;RelativeTimeuses deterministicdate.toISOString()fallback to fix SSR/CSR hydration mismatch.e224a77b,5f687c28,f06b7aa8).a4be2cd4.Test plan
Tracking