PR_26171_033 idea board inline accordion cell - #8
Conversation
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:e4ceb22261
ℹ️ 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".
| const chevron = document.createElement("img"); | ||
| chevron.alt = ""; | ||
| chevron.dataset.ideaBoardChevron = record.ideaId; | ||
| chevron.src = expanded ? "assets/theme-v2/images/gfs-chevron-up.svg" : "assets/theme-v2/images/gfs-chevron-down.svg"; |
There was a problem hiding this comment.
On the Idea Board page this new chevron is rendered inside .tool-center-panel, whose existing theme rule assets/theme-v2/css/panels.css:245 applies to every descendant img with a large centered block layout (width: min(var(--tool-center-image-max), 75%), margins, etc.). As a result each idea row gets a large block image instead of an inline 16px chevron whenever the table renders; give this icon explicit sizing/class styling or avoid matching the generic panel image rule.
Useful? React with 👍 / 👎.
| idea.dataset.ideaBoardToggleNotes = record.ideaId; | ||
| idea.dataset.ideaBoardIdeaCell = record.ideaId; | ||
| idea.setAttribute("aria-expanded", String(expanded)); | ||
| idea.setAttribute("role", "button"); |
There was a problem hiding this comment.
For screen-reader users navigating this table, assigning role="button" to the <th scope="row"> overrides its native row-header semantics, so the Pitch/Status/Notes cells are no longer associated with the idea name as their row label. The cell can still own the toggle by keeping the <th> semantics and placing/styling a full-cell button or equivalent control inside it.
Useful? React with 👍 / 👎.
| @@ -0,0 +1,3 @@ | |||
| <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 16 16" aria-hidden="true"> | |||
| <path fill="currentColor" d="M3.2 5.4 8 10.2l4.8-4.8 1.1 1.1L8 12.4 2.1 6.5z"/> | |||
There was a problem hiding this comment.
When these SVGs are loaded through an <img> tag, currentColor is resolved inside the standalone image document rather than inheriting the .data-table th gold text color, so the new chevrons render with the browser default color instead of the table/theme color. On this dark themed Idea Board surface that makes the expand affordance low-contrast; use an explicit themed fill or inline/mask styling that can inherit color.
Useful? React with 👍 / 👎.
Summary
Validation
node --check toolbox/idea-board/index.jsnode --check tests/playwright/tools/IdeaBoardTableNotes.spec.mjsnode --check tests/playwright/tools/ToolboxRoutePages.spec.mjsnpx playwright test tests/playwright/tools/IdeaBoardTableNotes.spec.mjs --project=playwright --workers=1 --reporter=linenpx playwright test tests/playwright/tools/ToolboxRoutePages.spec.mjs --project=playwright --workers=1 --reporter=line -g "Idea Board launches"Artifact
tmp/PR_26171_033-idea-board-inline-accordion-cell_delta.zip