PR_26179_ALFA_010-tool-display-single-line-summary#254
Conversation
ToolboxAid
deleted the
PR_26179_ALFA_010-tool-display-single-line-summary
branch
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2b9d1e655b
ℹ️ 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".
| body.appendChild(identityRow); | ||
| displayMode.appendChild(body); | ||
| slot.replaceWith(displayMode); | ||
| summary.appendChild(character); |
There was a problem hiding this comment.
When a tool page is viewed on common phone widths, this puts the fixed 225px character into the same non-wrapping summary row as the 64px badge, ~42px fullscreen icon, three 10px gaps, and 20px padding, so the row needs about 381px before the tool name and overflows a 390px viewport's ~354px container. The previous layout kept the character out of the summary row, so this regresses mobile tool pages; add a narrow-viewport rule to shrink/hide the character or otherwise let the row fit.
Useful? React with 👍 / 👎.
| toolNameLabel.className = "tool-display-mode__tool-name"; | ||
| toolNameLabel.textContent = toolName; | ||
| summary.appendChild(toolNameLabel); |
There was a problem hiding this comment.
For screen-reader users, this new visible tool name is not exposed because the parent summary still has aria-label="Tool Display Mode", which takes precedence over descendant text when computing the control name. Since this PR also removes the old description/body markup, the fullscreen summary is now announced generically on every tool page instead of identifying the current tool; include the tool name in the summary label or label it from this text node.
Useful? React with 👍 / 👎.
| const __filename = fileURLToPath(import.meta.url); | ||
| const __dirname = path.dirname(__filename); | ||
| const repoRoot = path.resolve(__dirname, "..", ".."); | ||
| const repoRoot = path.resolve(__dirname, "..", "..", ".."); |
There was a problem hiding this comment.
With the server root moved from dev/ to the repository root, the generic static-file path now resolves and returns any file under the repo, so a request such as /.env will be served whenever local credentials are present. This helper binds to localhost, but during Playwright/dev runs any local page or process can read those values; keep the root fix while denying dot/env files before readFile.
Useful? React with 👍 / 👎.
2 participants
PR_26179_ALFA_010-tool-display-single-line-summary Report
Purpose
Recreate the useful Tool Display Mode layout scope from stale PR #196 and fold in the validation intent from PR #198 without recreating #198 as a separate PR.
Product Outcome
Tool Display Mode now renders a single-line summary where the summary directly owns, in order:
The deprecated Tool Display Mode body, chevron, identity row, and previous/next navigation row are no longer generated by the shared Theme V2 Tool Display Mode script.
Scope Notes
dev/tests/playwright/tools/.dev/tests/helpers/playwrightRepoServer.mjsto resolve the repository root from the currentdev/tests/location so current Playwright tests serve production pages instead ofdev/.docs_build/report paths ortmp/ZIP paths.Changed File Summary
assets/theme-v2/js/tool-display-mode.js- updated single-line summary generation and removed deprecated navigation row generation.assets/theme-v2/css/panels.css- updated Tool Display Mode layout styling.assets/theme-v2/css/accordion.css- removed obsolete Tool Display Mode chevron styling from accordion icon selectors.dev/tests/playwright/tools/ToolDisplayModeSingleLineSummary.spec.mjs- added current targeted Tool Display Mode coverage.dev/tests/playwright/tools/ToolDisplayModeNavigation.spec.mjs- removed stale two-row navigation spec.dev/tests/playwright/tools/ToolNavigationPrevNext.spec.mjs- updated adjacent navigation assertions to match the removed Tool Display Mode controls.dev/tests/playwright/tools/ToolImageRegistry.spec.mjs- updated image assertions for the direct summary layout.dev/tests/playwright/tools/ToolboxSelectedGameStatusBar.spec.mjs- updated status-bar assertions for removed Tool Display Mode navigation controls.dev/tests/playwright/tools/IdeaBoardTableNotes.spec.mjs- removed the obsolete expectation that navigation warning output is required.dev/tests/helpers/playwrightRepoServer.mjs- corrected repo-root resolution for tests underdev/tests/.dev/scripts/run-targeted-test-lanes.mjs- updated targeted lane metadata and spec path.Validation Summary
npm run test:lane:tool-display-modePASS, 3/3.git diff --checkPASS.npm run validate:canonical-structurePASS.Outcome ZIP
dev/workspace/zips/PR_26179_ALFA_010-tool-display-single-line-summary_delta.zipNotes
The old
assets/theme-v2/css/gamefoundrystudio.cssfile still contains historical Tool Display Mode selectors, but it is not imported by the active Theme V2theme.css. This PR leaves that legacy residue untouched to avoid unrelated cleanup.