Skip to content

Keep Review content in the selected Paper theme - #2821

Merged
Chris0Jeky merged 11 commits into
mainfrom
issue-2009/review-theme-inheritance
Sep 9, 2026
Merged

Keep Review content in the selected Paper theme#2821
Chris0Jeky merged 11 commits into
mainfrom
issue-2009/review-theme-inheritance

Conversation

@Chris0Jeky

@Chris0Jeky Chris0Jeky commented Sep 9, 2026

Copy link
Copy Markdown
Owner

Review's nested .paper class resets selected night tokens to light. Removing that reset makes Review inherit the selected theme. Shared typography, spacing, radius and motion tokens now apply to both .paper and .paper-night, so night Review retains its structural styles while using night colours.

Refs #2009. The rendered palette comparison is delivered in #2820; D6 palette selection remains a maintainer choice. This change reduces visual friction without changing palette colour values or review/Apply behavior.

Validation at fix head 5ee00c4bf34923640976318d79cf8e69b7663431, base 22fcac405946e226c136e2fd52be3b8cdf9ecca7:

  • Populated Chromium light/night regression: the original reset fails night colour parity; removing it exposed missing structural tokens (heading16px/button16px/radius0px/transition0s). With shared tokens, both cases pass and retain44px/12px/4px/140ms. Final2passed/15.2s on an isolated synthetic Mock stack; ports58749/5187 stopped.
  • Typecheck, scoped ESLint, build, doc links (674files/0broken), and diff check pass.
  • Earlier unchanged application logic: PaperReviewView214 passed; full frontend394files/6243passed/3skipped at the source implementation. Full Vitest was not repeated for the CSS fix; computed-style browser assertions exercise that seam.
  • Original and fresh fix Terra reviews completed SHIP; the confirmed structural-token HIGH is closed. Required hosted CI is requalifying the new head, and its aging window reset.

Three scoped files: Review root class, shared token CSS, and the light/night E2E test. This source is preserved in sound integration #2826; parked #2824 remains excluded. OUTSTANDING_TASKS.md remains unchanged; #2009 stays open for its remaining work. Existing build dynamic-import warning remains unchanged.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 9, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-09T12:48:55.186421Z 5ee00c4 New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@Chris0Jeky

Copy link
Copy Markdown
Owner Author

Independent Terra high review of 85f064d3e879d90623dc8a1e8cfd72c502bcf333 against 22029c760653519622de2e9bcdd86bb668a50d41: SHIP, no confirmed HIGH/CRITICAL findings. Removing the nested .paper reset preserves scoped layout and inherits the active body theme; no compound selector relies on the removed class. The browser regression checks a real seeded proposal, four inherited tokens and rendered background in both modes. Reviewer inspected source and tests read-only; coordinator evidence is the recorded red/green browser run, full frontend suite, typecheck, build and ESLint. D-6 palette adoption remains pending.

@Chris0Jeky

Copy link
Copy Markdown
Owner Author

Closeout lane release: #2819 has landed at843a8f6cba22417a030e9f2e3143c3469fc1aaf7 with required CI green. Existing-PR closeout and canonical delivery-doc lease are complete. This later PR remains owned by its author lane; refresh/reconcile against current main before merge under the normal gate.

@Chris0Jeky

Copy link
Copy Markdown
Owner Author

Current-base qualification update: merged main843a8f6cba22417a030e9f2e3143c3469fc1aaf7, new head66e5eacdb280771132a6d3eb48246e4a7bc998e6. Base delta is the independently reviewed unused dispatcher deletion plus delivery docs. Source patch is byte-equivalent by stable patch-id0dd1dc22e354889a7f1ee1b11d294e74518c3aab; no theme or browser-test logic changed. Current-base PaperReviewView214 tests and typecheck pass; diff clean. Existing source review remains applicable. Full frontend and browser results remain scoped to the prior source head; new hosted qualification is required before merge.

@Chris0Jeky

Copy link
Copy Markdown
Owner Author

Integration candidate #2824 is ready at ee90123. This source head and its owned files are preserved exactly. Combined local qualification passed: backend9092/5skip with aggregateexit0, frontend6234/3skip, Chromium4, build/typecheck/lint/docslinks. The integration introduces no source edits and retains the completed source review/fix records.

Please hold individual merges while #2824 qualifies against current main. This source PR remains open; it will be closed as superseded only after actual integration delivery and exact merge-tree proof. No parent issue or human acceptance is closed early.

@Chris0Jeky

Copy link
Copy Markdown
Owner Author

Worktree cleanup complete: clean tracked state and source HEAD ancestry in pushed integration #2824 (ee90123) were verified; the owned source worktree was removed with plain git worktree remove. Unique ignored runtime/test artifacts were copied OUT first to C:\Users\jekyt\source\Taskdeck.claude\local\milestone-20260909\preserved-2821. Source branch refs and commits remain preserved; this PR stays open pending integration qualification and review disposition. Primary checkout and unrelated worktrees were preserved.

@Chris0Jeky

Copy link
Copy Markdown
Owner Author

Current-base qualification: #2820 delivered at main22fcac405946e226c136e2fd52be3b8cdf9ecca7. #2821 refreshed without conflict to099e41c4c9b3b0c4eebd5e7615420e4b00fdf1ac, preserving source66e5eacdb and exactly its two-file feature diff. Entire frontend tree matches the reviewed source; base delta is the separately reviewed palette evidence package. Current-base PaperReviewView214 tests, typecheck, doclinks674/0broken and diffcheck pass. Prior real light/night browser2pass and full-suite evidence remain applicable; no new behavior introduced by this merge. Hosted CI and head aging reset; this source will deliver independently of held integration #2824. The first targeted invocation used a nonexistent test path and ran no tests; corrected path produced the214-pass result.

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 099e41c4c9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread frontend/taskdeck-web/src/views/paper/PaperReviewView.vue
@Chris0Jeky

Copy link
Copy Markdown
Owner Author

Confirmed HIGH / fix-now in the first fix round: removing the nested .paper class leaves body.paper-night without shared type/spacing/radius/motion tokens. Terra exercised actual CSS in Chromium: heading44px ->16px; button12px ->16px, radius4px ->0px, transition140ms ->0s. Night colours were correct but structural styles regressed. One bounded fix batch is active: share only theme-independent declarations between.paper and.paper-night and extend populated light/night browser proof with representative structural assertions. Separate light/night colour/shadow tokens stay distinct. No merge until fix checks, fresh risk-boundary review, hosted qualification and aging pass.

@Chris0Jeky

Copy link
Copy Markdown
Owner Author

Fix round pushed at5ee00c4bf34923640976318d79cf8e69b7663431: move exactly the shared scale/spacing/radius/motion declarations into.paper,.paper-night, keeping distinct colour/shadow declarations. Populated Chromium regression red first (light passes, night16px/16px/0px/0s), then green2passed/15.2s with expected44px/12px/4px/140ms. Typecheck/scopedESLint/build/doclinks pass; ports58749/5187 stopped. Full Vitest was not repeated for this CSS-only fix because browser computed styles exercise the changed seam. Fresh Terra fix review is running; required hosted qualification and age reset. This is the one source fix batch.

@Chris0Jeky

Copy link
Copy Markdown
Owner Author

Fresh Terra fix review SHIP at5ee00c4bf34923640976318d79cf8e69b7663431: shared tokens restore both modes without overriding later narrow scaling or Grove radius specificity; existing reduced-motion behavior remains unchanged. Populated browser structural assertions cover both modes. No new HIGH/CRITICAL or lower finding in the bounded fix. Confirmed token-loss HIGH is closed; source review/fix pipeline complete pending hosted qualification and aging.

@Chris0Jeky

Copy link
Copy Markdown
Owner Author

Final combined candidate #2826 atf7e65bfbddccbf197ea3c7d258b05ef9418a4c6a preserves this reviewed source and passes fullfrontend6234/3skip, combinedChromium5, typecheck/build/lint/doclinks, and freshTerraSHIP. Holding individual merge while that exact combined tree qualifies. This replaces the earlier separate-delivery plan; parked2823/2824 remain excluded. Keep source open until integration delivery and source-content proof.

@Chris0Jeky

Copy link
Copy Markdown
Owner Author

Completed fix qualification worktree removed after clean state, remote-head preservation and no-active-process checks. Unique .tmp runtime/review evidence and frontend test-results were copied OUT to C:\Users\jekyt\source\Taskdeck.claude\local\milestone-20260909\preserved-2821-fix before plain git worktree remove. Original source evidence remains in preserved-2821. Source5ee00c4bf and combined2826 history remain remote; no unique material was discarded.

@Chris0Jeky
Chris0Jeky merged commit 474cc4f into main Sep 9, 2026
39 checks passed
@Chris0Jeky
Chris0Jeky deleted the issue-2009/review-theme-inheritance branch September 9, 2026 13:27
@github-project-automation github-project-automation Bot moved this from Review to Done in Taskdeck Execution Sep 9, 2026
@Chris0Jeky

Copy link
Copy Markdown
Owner Author

Delivered through integration #2826, merged as 06bd4d1. Coordinator verified the actual main tree equals qualified f7e65bf exactly (tree 7315d28ec3a2e709da4f701f4489a864ec2c36e2), and all three source heads are ancestors. GitHub automatically marked the source PRs merged through the preserved merge commits. Required ci-required.yml run34353507971 passed, independent Terra SHIP, no unresolved threads. Previously tracked MEDIUM limitations remain follow-ups; no full parent-issue completion claim.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant