fix(desktop): align project session titles with the project name - #3311

Merged
Astro-Han merged 2 commits into
mainfrom
fix/sidebar-align-session-titles
Aug 20, 2026
Merged

fix(desktop): align project session titles with the project name#3311
Astro-Han merged 2 commits into
mainfrom
fix/sidebar-align-session-titles

Conversation

@Astro-Han

@Astro-HanAstro-Han commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Project-grouped session rows were nested a full Astryx SideNav step (24px). That step is for icon-less children so their text meets a parent title after a 16px icon + 8px gap. Session rows already spend 8px on StatusDot in that slot, so the extra 24px pushed titles too far right of the project name.

image

Keep the remaining 8px nest (--spacing-2) so titles share one x and the row still sits inside the project.

Verification

  • Ran node --experimental-strip-types --test apps/desktop/src/main/__tests__/session-project-hierarchy-contract.test.ts in the worktree (pass).
  • Did not run Playwright locally: the geometry spec opens a visible Electron window and would steal focus from an active maka dev session. Leave sidebar-project-row.spec.ts to CI.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope:

Maka diagnosed the indent, chose the 8px remaining nest, and authored the CSS, contract test, e2e assertion, and commit.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

Astryx nests icon-less children by 24px so their text meets a parent
title after a 16px icon + 8px gap. Session rows already spend 8px on
StatusDot in that slot, so keep the remaining 8px nest instead of a
full SideNav step.
Generated-by: Maka
@likun666661

likun666661 commented Aug 20, 2026

Copy link
Copy Markdown
Member

Overall, the direction is sound and the spacing-6 → spacing-2 geometry checks out. Before marking this PR ready, however, I suggest tightening both the problem definition and the E2E contract.

The current geometry is:

  • Project title: 8px row padding + 16px Folder icon + 8px gap = 32px
  • Session title before this PR: 24px child inset + 8px row padding + 8px StatusDot + 8px gap = 48px
  • Session title after this PR: 8px child inset + 8px row padding + 8px StatusDot + 8px gap = 32px

So the old layout does place session titles 16px too far to the right, and --spacing-2 aligns them exactly. The production change itself is minimal and appropriate.

I see three points worth addressing:

  1. The problem statement presents a product choice as a design-system fact. The claim that the 24px SideNav nesting step is specifically for icon-less children is not established by the current Astryx implementation; Astryx applies spacing-6 to children unconditionally. A more precise statement would be: “The parent and child rows have leading content of different widths. Combined with the fixed nesting inset, this misaligns their titles. The product intent is to align the titles while retaining an 8px hierarchical inset.” This matters because fix(desktop): indent project session rows #3175 only recently defined the full 24px nesting step as the visual contract. This PR is selecting a different contract, not merely correcting arithmetic. A before/after screenshot or design reference would make that intent explicit.

  2. The E2E assertion does not directly verify the stated result. It currently checks that the project and session button left edges differ by 6–16px, rather than checking that their title x-coordinates match. If the StatusDot width, icon size, or SideNav gap changes later, the titles could become misaligned while the test still passes. I suggest directly asserting abs(projectTitle.x - sessionTitle.x) <= 1–2px. If preserving hierarchy is also a contract, separately assert that the session button begins about 8px inside the project button.

  3. The visible effect is broader than the title. This change moves the left edge of the entire session button, including its hover/selected background and hit area, 16px to the left. The right edge should remain stable. That is likely acceptable, but it should be included in the behavior description.

From an Occam’s razor perspective, I would keep the current spacing-2 implementation. A calc() expression or a new component abstraction would add complexity without improving the result. The main improvement is to describe the actual product decision and test the visual outcome—title alignment—rather than pinning a CSS token or internal DOM mechanism.

AI-assisted review disclosure: This comment was prepared with Codex assistance after statically reviewing the PR diff, the Astryx SideNav/StatusDot layout, and the history of #3175.

Keep the 8px nest. State it as a product choice against SideNav's fixed
24px child inset, and check title x in the sidebar e2e instead of only
the session button edge.
Generated-by: Maka
@Astro-Han

Copy link
Copy Markdown
ContributorAuthor

Accepted points 1–3. Kept --spacing-2; that is still the smallest implementation.

  1. The 24px SideNav nest is a fixed child inset, not an icon-less-child alignment rule. This PR chooses a different product contract from fix(desktop): indent project session rows #3175: keep an 8px hierarchical inset so session titles share the project title's x. Comments in sidebar.css, the source contract, and the ProjectGroups story now say that.

  2. The sidebar e2e now asserts abs(projectTitle.x - sessionTitle.x) <= 2 and still checks that the session button starts about 8px inside the project button.

  3. The whole session button, including hover/selected fill and hit area, moves 16px left; the right edge stays on the rail. That is now in the CSS comment.

Before/after for the title column is on the desktop compare image from the earlier thread.

@Astro-Han
Astro-Han marked this pull request as ready for review August 20, 2026 14:56
@Astro-Han
Astro-Han requested a review from M4n5terAugust 20, 2026 14:56

@hqhq1025hqhq1025 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the exact current revision against the stated layout contract, production DOM/CSS path, prior maintainer feedback, and focused geometry/Electron coverage. No actionable defect survived the policy v7 materiality review.

Codex-assisted review performed under the maintainer-approved review workflow.

@Astro-Han
Astro-Han merged commit db0432e into mainAug 20, 2026
1 check passed
@Astro-Han
Astro-Han deleted the fix/sidebar-align-session-titles branch August 20, 2026 19:40
Sign up for freeto 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.

3 participants

@Astro-Han@likun666661@hqhq1025
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

fix(desktop): align project session titles with the project name - #3311

Merged
Astro-Han merged 2 commits into
mainfrom
fix/sidebar-align-session-titles
Aug 20, 2026
Merged

fix(desktop): align project session titles with the project name#3311
Astro-Han merged 2 commits into
mainfrom
fix/sidebar-align-session-titles

Conversation

@Astro-Han

@Astro-HanAstro-Han commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Project-grouped session rows were nested a full Astryx SideNav step (24px). That step is for icon-less children so their text meets a parent title after a 16px icon + 8px gap. Session rows already spend 8px on StatusDot in that slot, so the extra 24px pushed titles too far right of the project name.

image

Keep the remaining 8px nest (--spacing-2) so titles share one x and the row still sits inside the project.

Verification

  • Ran node --experimental-strip-types --test apps/desktop/src/main/__tests__/session-project-hierarchy-contract.test.ts in the worktree (pass).
  • Did not run Playwright locally: the geometry spec opens a visible Electron window and would steal focus from an active maka dev session. Leave sidebar-project-row.spec.ts to CI.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope:

Maka diagnosed the indent, chose the 8px remaining nest, and authored the CSS, contract test, e2e assertion, and commit.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

Astryx nests icon-less children by 24px so their text meets a parent
title after a 16px icon + 8px gap. Session rows already spend 8px on
StatusDot in that slot, so keep the remaining 8px nest instead of a
full SideNav step.
Generated-by: Maka
@likun666661

likun666661 commented Aug 20, 2026

Copy link
Copy Markdown
Member

Overall, the direction is sound and the spacing-6 → spacing-2 geometry checks out. Before marking this PR ready, however, I suggest tightening both the problem definition and the E2E contract.

The current geometry is:

  • Project title: 8px row padding + 16px Folder icon + 8px gap = 32px
  • Session title before this PR: 24px child inset + 8px row padding + 8px StatusDot + 8px gap = 48px
  • Session title after this PR: 8px child inset + 8px row padding + 8px StatusDot + 8px gap = 32px

So the old layout does place session titles 16px too far to the right, and --spacing-2 aligns them exactly. The production change itself is minimal and appropriate.

I see three points worth addressing:

  1. The problem statement presents a product choice as a design-system fact. The claim that the 24px SideNav nesting step is specifically for icon-less children is not established by the current Astryx implementation; Astryx applies spacing-6 to children unconditionally. A more precise statement would be: “The parent and child rows have leading content of different widths. Combined with the fixed nesting inset, this misaligns their titles. The product intent is to align the titles while retaining an 8px hierarchical inset.” This matters because fix(desktop): indent project session rows #3175 only recently defined the full 24px nesting step as the visual contract. This PR is selecting a different contract, not merely correcting arithmetic. A before/after screenshot or design reference would make that intent explicit.

  2. The E2E assertion does not directly verify the stated result. It currently checks that the project and session button left edges differ by 6–16px, rather than checking that their title x-coordinates match. If the StatusDot width, icon size, or SideNav gap changes later, the titles could become misaligned while the test still passes. I suggest directly asserting abs(projectTitle.x - sessionTitle.x) <= 1–2px. If preserving hierarchy is also a contract, separately assert that the session button begins about 8px inside the project button.

  3. The visible effect is broader than the title. This change moves the left edge of the entire session button, including its hover/selected background and hit area, 16px to the left. The right edge should remain stable. That is likely acceptable, but it should be included in the behavior description.

From an Occam’s razor perspective, I would keep the current spacing-2 implementation. A calc() expression or a new component abstraction would add complexity without improving the result. The main improvement is to describe the actual product decision and test the visual outcome—title alignment—rather than pinning a CSS token or internal DOM mechanism.

AI-assisted review disclosure: This comment was prepared with Codex assistance after statically reviewing the PR diff, the Astryx SideNav/StatusDot layout, and the history of #3175.

Keep the 8px nest. State it as a product choice against SideNav's fixed
24px child inset, and check title x in the sidebar e2e instead of only
the session button edge.
Generated-by: Maka
@Astro-Han

Copy link
Copy Markdown
ContributorAuthor

Accepted points 1–3. Kept --spacing-2; that is still the smallest implementation.

  1. The 24px SideNav nest is a fixed child inset, not an icon-less-child alignment rule. This PR chooses a different product contract from fix(desktop): indent project session rows #3175: keep an 8px hierarchical inset so session titles share the project title's x. Comments in sidebar.css, the source contract, and the ProjectGroups story now say that.

  2. The sidebar e2e now asserts abs(projectTitle.x - sessionTitle.x) <= 2 and still checks that the session button starts about 8px inside the project button.

  3. The whole session button, including hover/selected fill and hit area, moves 16px left; the right edge stays on the rail. That is now in the CSS comment.

Before/after for the title column is on the desktop compare image from the earlier thread.

@Astro-Han
Astro-Han marked this pull request as ready for review August 20, 2026 14:56
@Astro-Han
Astro-Han requested a review from M4n5terAugust 20, 2026 14:56

@hqhq1025hqhq1025 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the exact current revision against the stated layout contract, production DOM/CSS path, prior maintainer feedback, and focused geometry/Electron coverage. No actionable defect survived the policy v7 materiality review.

Codex-assisted review performed under the maintainer-approved review workflow.

@Astro-Han
Astro-Han merged commit db0432e into mainAug 20, 2026
1 check passed
@Astro-Han
Astro-Han deleted the fix/sidebar-align-session-titles branch August 20, 2026 19:40
Sign up for freeto 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.

3 participants

@Astro-Han@likun666661@hqhq1025
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(desktop): align project session titles with the project name - #3311

Merged
Astro-Han merged 2 commits into
mainfrom
fix/sidebar-align-session-titles
Aug 20, 2026
Merged

fix(desktop): align project session titles with the project name#3311
Astro-Han merged 2 commits into
mainfrom
fix/sidebar-align-session-titles

Conversation

@Astro-Han

@Astro-HanAstro-Han commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Project-grouped session rows were nested a full Astryx SideNav step (24px). That step is for icon-less children so their text meets a parent title after a 16px icon + 8px gap. Session rows already spend 8px on StatusDot in that slot, so the extra 24px pushed titles too far right of the project name.

image

Keep the remaining 8px nest (--spacing-2) so titles share one x and the row still sits inside the project.

Verification

  • Ran node --experimental-strip-types --test apps/desktop/src/main/__tests__/session-project-hierarchy-contract.test.ts in the worktree (pass).
  • Did not run Playwright locally: the geometry spec opens a visible Electron window and would steal focus from an active maka dev session. Leave sidebar-project-row.spec.ts to CI.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope:

Maka diagnosed the indent, chose the 8px remaining nest, and authored the CSS, contract test, e2e assertion, and commit.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

Astryx nests icon-less children by 24px so their text meets a parent
title after a 16px icon + 8px gap. Session rows already spend 8px on
StatusDot in that slot, so keep the remaining 8px nest instead of a
full SideNav step.
Generated-by: Maka
@likun666661

likun666661 commented Aug 20, 2026

Copy link
Copy Markdown
Member

Overall, the direction is sound and the spacing-6 → spacing-2 geometry checks out. Before marking this PR ready, however, I suggest tightening both the problem definition and the E2E contract.

The current geometry is:

  • Project title: 8px row padding + 16px Folder icon + 8px gap = 32px
  • Session title before this PR: 24px child inset + 8px row padding + 8px StatusDot + 8px gap = 48px
  • Session title after this PR: 8px child inset + 8px row padding + 8px StatusDot + 8px gap = 32px

So the old layout does place session titles 16px too far to the right, and --spacing-2 aligns them exactly. The production change itself is minimal and appropriate.

I see three points worth addressing:

  1. The problem statement presents a product choice as a design-system fact. The claim that the 24px SideNav nesting step is specifically for icon-less children is not established by the current Astryx implementation; Astryx applies spacing-6 to children unconditionally. A more precise statement would be: “The parent and child rows have leading content of different widths. Combined with the fixed nesting inset, this misaligns their titles. The product intent is to align the titles while retaining an 8px hierarchical inset.” This matters because fix(desktop): indent project session rows #3175 only recently defined the full 24px nesting step as the visual contract. This PR is selecting a different contract, not merely correcting arithmetic. A before/after screenshot or design reference would make that intent explicit.

  2. The E2E assertion does not directly verify the stated result. It currently checks that the project and session button left edges differ by 6–16px, rather than checking that their title x-coordinates match. If the StatusDot width, icon size, or SideNav gap changes later, the titles could become misaligned while the test still passes. I suggest directly asserting abs(projectTitle.x - sessionTitle.x) <= 1–2px. If preserving hierarchy is also a contract, separately assert that the session button begins about 8px inside the project button.

  3. The visible effect is broader than the title. This change moves the left edge of the entire session button, including its hover/selected background and hit area, 16px to the left. The right edge should remain stable. That is likely acceptable, but it should be included in the behavior description.

From an Occam’s razor perspective, I would keep the current spacing-2 implementation. A calc() expression or a new component abstraction would add complexity without improving the result. The main improvement is to describe the actual product decision and test the visual outcome—title alignment—rather than pinning a CSS token or internal DOM mechanism.

AI-assisted review disclosure: This comment was prepared with Codex assistance after statically reviewing the PR diff, the Astryx SideNav/StatusDot layout, and the history of #3175.

Keep the 8px nest. State it as a product choice against SideNav's fixed
24px child inset, and check title x in the sidebar e2e instead of only
the session button edge.
Generated-by: Maka
@Astro-Han

Copy link
Copy Markdown
ContributorAuthor

Accepted points 1–3. Kept --spacing-2; that is still the smallest implementation.

  1. The 24px SideNav nest is a fixed child inset, not an icon-less-child alignment rule. This PR chooses a different product contract from fix(desktop): indent project session rows #3175: keep an 8px hierarchical inset so session titles share the project title's x. Comments in sidebar.css, the source contract, and the ProjectGroups story now say that.

  2. The sidebar e2e now asserts abs(projectTitle.x - sessionTitle.x) <= 2 and still checks that the session button starts about 8px inside the project button.

  3. The whole session button, including hover/selected fill and hit area, moves 16px left; the right edge stays on the rail. That is now in the CSS comment.

Before/after for the title column is on the desktop compare image from the earlier thread.

@Astro-Han
Astro-Han marked this pull request as ready for review August 20, 2026 14:56
@Astro-Han
Astro-Han requested a review from M4n5terAugust 20, 2026 14:56

@hqhq1025hqhq1025 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the exact current revision against the stated layout contract, production DOM/CSS path, prior maintainer feedback, and focused geometry/Electron coverage. No actionable defect survived the policy v7 materiality review.

Codex-assisted review performed under the maintainer-approved review workflow.

@Astro-Han
Astro-Han merged commit db0432e into mainAug 20, 2026
1 check passed
@Astro-Han
Astro-Han deleted the fix/sidebar-align-session-titles branch August 20, 2026 19:40
Sign up for freeto 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.

3 participants

@Astro-Han@likun666661@hqhq1025
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(desktop): align project session titles with the project name - #3311

Merged
Astro-Han merged 2 commits into
mainfrom
fix/sidebar-align-session-titles
Aug 20, 2026
Merged

fix(desktop): align project session titles with the project name#3311
Astro-Han merged 2 commits into
mainfrom
fix/sidebar-align-session-titles

Conversation

@Astro-Han

@Astro-HanAstro-Han commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Project-grouped session rows were nested a full Astryx SideNav step (24px). That step is for icon-less children so their text meets a parent title after a 16px icon + 8px gap. Session rows already spend 8px on StatusDot in that slot, so the extra 24px pushed titles too far right of the project name.

image

Keep the remaining 8px nest (--spacing-2) so titles share one x and the row still sits inside the project.

Verification

  • Ran node --experimental-strip-types --test apps/desktop/src/main/__tests__/session-project-hierarchy-contract.test.ts in the worktree (pass).
  • Did not run Playwright locally: the geometry spec opens a visible Electron window and would steal focus from an active maka dev session. Leave sidebar-project-row.spec.ts to CI.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope:

Maka diagnosed the indent, chose the 8px remaining nest, and authored the CSS, contract test, e2e assertion, and commit.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

Astryx nests icon-less children by 24px so their text meets a parent
title after a 16px icon + 8px gap. Session rows already spend 8px on
StatusDot in that slot, so keep the remaining 8px nest instead of a
full SideNav step.
Generated-by: Maka
@likun666661

likun666661 commented Aug 20, 2026

Copy link
Copy Markdown
Member

Overall, the direction is sound and the spacing-6 → spacing-2 geometry checks out. Before marking this PR ready, however, I suggest tightening both the problem definition and the E2E contract.

The current geometry is:

  • Project title: 8px row padding + 16px Folder icon + 8px gap = 32px
  • Session title before this PR: 24px child inset + 8px row padding + 8px StatusDot + 8px gap = 48px
  • Session title after this PR: 8px child inset + 8px row padding + 8px StatusDot + 8px gap = 32px

So the old layout does place session titles 16px too far to the right, and --spacing-2 aligns them exactly. The production change itself is minimal and appropriate.

I see three points worth addressing:

  1. The problem statement presents a product choice as a design-system fact. The claim that the 24px SideNav nesting step is specifically for icon-less children is not established by the current Astryx implementation; Astryx applies spacing-6 to children unconditionally. A more precise statement would be: “The parent and child rows have leading content of different widths. Combined with the fixed nesting inset, this misaligns their titles. The product intent is to align the titles while retaining an 8px hierarchical inset.” This matters because fix(desktop): indent project session rows #3175 only recently defined the full 24px nesting step as the visual contract. This PR is selecting a different contract, not merely correcting arithmetic. A before/after screenshot or design reference would make that intent explicit.

  2. The E2E assertion does not directly verify the stated result. It currently checks that the project and session button left edges differ by 6–16px, rather than checking that their title x-coordinates match. If the StatusDot width, icon size, or SideNav gap changes later, the titles could become misaligned while the test still passes. I suggest directly asserting abs(projectTitle.x - sessionTitle.x) <= 1–2px. If preserving hierarchy is also a contract, separately assert that the session button begins about 8px inside the project button.

  3. The visible effect is broader than the title. This change moves the left edge of the entire session button, including its hover/selected background and hit area, 16px to the left. The right edge should remain stable. That is likely acceptable, but it should be included in the behavior description.

From an Occam’s razor perspective, I would keep the current spacing-2 implementation. A calc() expression or a new component abstraction would add complexity without improving the result. The main improvement is to describe the actual product decision and test the visual outcome—title alignment—rather than pinning a CSS token or internal DOM mechanism.

AI-assisted review disclosure: This comment was prepared with Codex assistance after statically reviewing the PR diff, the Astryx SideNav/StatusDot layout, and the history of #3175.

Keep the 8px nest. State it as a product choice against SideNav's fixed
24px child inset, and check title x in the sidebar e2e instead of only
the session button edge.
Generated-by: Maka
@Astro-Han

Copy link
Copy Markdown
ContributorAuthor

Accepted points 1–3. Kept --spacing-2; that is still the smallest implementation.

  1. The 24px SideNav nest is a fixed child inset, not an icon-less-child alignment rule. This PR chooses a different product contract from fix(desktop): indent project session rows #3175: keep an 8px hierarchical inset so session titles share the project title's x. Comments in sidebar.css, the source contract, and the ProjectGroups story now say that.

  2. The sidebar e2e now asserts abs(projectTitle.x - sessionTitle.x) <= 2 and still checks that the session button starts about 8px inside the project button.

  3. The whole session button, including hover/selected fill and hit area, moves 16px left; the right edge stays on the rail. That is now in the CSS comment.

Before/after for the title column is on the desktop compare image from the earlier thread.

@Astro-Han
Astro-Han marked this pull request as ready for review August 20, 2026 14:56
@Astro-Han
Astro-Han requested a review from M4n5terAugust 20, 2026 14:56

@hqhq1025hqhq1025 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the exact current revision against the stated layout contract, production DOM/CSS path, prior maintainer feedback, and focused geometry/Electron coverage. No actionable defect survived the policy v7 materiality review.

Codex-assisted review performed under the maintainer-approved review workflow.

@Astro-Han
Astro-Han merged commit db0432e into mainAug 20, 2026
1 check passed
@Astro-Han
Astro-Han deleted the fix/sidebar-align-session-titles branch August 20, 2026 19:40
Sign up for freeto 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.

3 participants

@Astro-Han@likun666661@hqhq1025
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

fix(desktop): align project session titles with the project name - #3311

Merged
Astro-Han merged 2 commits into
mainfrom
fix/sidebar-align-session-titles
Aug 20, 2026
Merged

fix(desktop): align project session titles with the project name#3311
Astro-Han merged 2 commits into
mainfrom
fix/sidebar-align-session-titles

Conversation

@Astro-Han

@Astro-HanAstro-Han commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Project-grouped session rows were nested a full Astryx SideNav step (24px). That step is for icon-less children so their text meets a parent title after a 16px icon + 8px gap. Session rows already spend 8px on StatusDot in that slot, so the extra 24px pushed titles too far right of the project name.

image

Keep the remaining 8px nest (--spacing-2) so titles share one x and the row still sits inside the project.

Verification

  • Ran node --experimental-strip-types --test apps/desktop/src/main/__tests__/session-project-hierarchy-contract.test.ts in the worktree (pass).
  • Did not run Playwright locally: the geometry spec opens a visible Electron window and would steal focus from an active maka dev session. Leave sidebar-project-row.spec.ts to CI.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope:

Maka diagnosed the indent, chose the 8px remaining nest, and authored the CSS, contract test, e2e assertion, and commit.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

Astryx nests icon-less children by 24px so their text meets a parent
title after a 16px icon + 8px gap. Session rows already spend 8px on
StatusDot in that slot, so keep the remaining 8px nest instead of a
full SideNav step.
Generated-by: Maka
@likun666661

likun666661 commented Aug 20, 2026

Copy link
Copy Markdown
Member

Overall, the direction is sound and the spacing-6 → spacing-2 geometry checks out. Before marking this PR ready, however, I suggest tightening both the problem definition and the E2E contract.

The current geometry is:

  • Project title: 8px row padding + 16px Folder icon + 8px gap = 32px
  • Session title before this PR: 24px child inset + 8px row padding + 8px StatusDot + 8px gap = 48px
  • Session title after this PR: 8px child inset + 8px row padding + 8px StatusDot + 8px gap = 32px

So the old layout does place session titles 16px too far to the right, and --spacing-2 aligns them exactly. The production change itself is minimal and appropriate.

I see three points worth addressing:

  1. The problem statement presents a product choice as a design-system fact. The claim that the 24px SideNav nesting step is specifically for icon-less children is not established by the current Astryx implementation; Astryx applies spacing-6 to children unconditionally. A more precise statement would be: “The parent and child rows have leading content of different widths. Combined with the fixed nesting inset, this misaligns their titles. The product intent is to align the titles while retaining an 8px hierarchical inset.” This matters because fix(desktop): indent project session rows #3175 only recently defined the full 24px nesting step as the visual contract. This PR is selecting a different contract, not merely correcting arithmetic. A before/after screenshot or design reference would make that intent explicit.

  2. The E2E assertion does not directly verify the stated result. It currently checks that the project and session button left edges differ by 6–16px, rather than checking that their title x-coordinates match. If the StatusDot width, icon size, or SideNav gap changes later, the titles could become misaligned while the test still passes. I suggest directly asserting abs(projectTitle.x - sessionTitle.x) <= 1–2px. If preserving hierarchy is also a contract, separately assert that the session button begins about 8px inside the project button.

  3. The visible effect is broader than the title. This change moves the left edge of the entire session button, including its hover/selected background and hit area, 16px to the left. The right edge should remain stable. That is likely acceptable, but it should be included in the behavior description.

From an Occam’s razor perspective, I would keep the current spacing-2 implementation. A calc() expression or a new component abstraction would add complexity without improving the result. The main improvement is to describe the actual product decision and test the visual outcome—title alignment—rather than pinning a CSS token or internal DOM mechanism.

AI-assisted review disclosure: This comment was prepared with Codex assistance after statically reviewing the PR diff, the Astryx SideNav/StatusDot layout, and the history of #3175.

Keep the 8px nest. State it as a product choice against SideNav's fixed
24px child inset, and check title x in the sidebar e2e instead of only
the session button edge.
Generated-by: Maka
@Astro-Han

Copy link
Copy Markdown
ContributorAuthor

Accepted points 1–3. Kept --spacing-2; that is still the smallest implementation.

  1. The 24px SideNav nest is a fixed child inset, not an icon-less-child alignment rule. This PR chooses a different product contract from fix(desktop): indent project session rows #3175: keep an 8px hierarchical inset so session titles share the project title's x. Comments in sidebar.css, the source contract, and the ProjectGroups story now say that.

  2. The sidebar e2e now asserts abs(projectTitle.x - sessionTitle.x) <= 2 and still checks that the session button starts about 8px inside the project button.

  3. The whole session button, including hover/selected fill and hit area, moves 16px left; the right edge stays on the rail. That is now in the CSS comment.

Before/after for the title column is on the desktop compare image from the earlier thread.

@Astro-Han
Astro-Han marked this pull request as ready for review August 20, 2026 14:56
@Astro-Han
Astro-Han requested a review from M4n5terAugust 20, 2026 14:56

@hqhq1025hqhq1025 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the exact current revision against the stated layout contract, production DOM/CSS path, prior maintainer feedback, and focused geometry/Electron coverage. No actionable defect survived the policy v7 materiality review.

Codex-assisted review performed under the maintainer-approved review workflow.

@Astro-Han
Astro-Han merged commit db0432e into mainAug 20, 2026
1 check passed
@Astro-Han
Astro-Han deleted the fix/sidebar-align-session-titles branch August 20, 2026 19:40
Sign up for freeto 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.

3 participants

@Astro-Han@likun666661@hqhq1025
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(desktop): align project session titles with the project name - #3311

Merged
Astro-Han merged 2 commits into
mainfrom
fix/sidebar-align-session-titles
Aug 20, 2026
Merged

fix(desktop): align project session titles with the project name#3311
Astro-Han merged 2 commits into
mainfrom
fix/sidebar-align-session-titles

Conversation

@Astro-Han

@Astro-HanAstro-Han commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Project-grouped session rows were nested a full Astryx SideNav step (24px). That step is for icon-less children so their text meets a parent title after a 16px icon + 8px gap. Session rows already spend 8px on StatusDot in that slot, so the extra 24px pushed titles too far right of the project name.

image

Keep the remaining 8px nest (--spacing-2) so titles share one x and the row still sits inside the project.

Verification

  • Ran node --experimental-strip-types --test apps/desktop/src/main/__tests__/session-project-hierarchy-contract.test.ts in the worktree (pass).
  • Did not run Playwright locally: the geometry spec opens a visible Electron window and would steal focus from an active maka dev session. Leave sidebar-project-row.spec.ts to CI.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope:

Maka diagnosed the indent, chose the 8px remaining nest, and authored the CSS, contract test, e2e assertion, and commit.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

Astryx nests icon-less children by 24px so their text meets a parent
title after a 16px icon + 8px gap. Session rows already spend 8px on
StatusDot in that slot, so keep the remaining 8px nest instead of a
full SideNav step.
Generated-by: Maka
@likun666661

likun666661 commented Aug 20, 2026

Copy link
Copy Markdown
Member

Overall, the direction is sound and the spacing-6 → spacing-2 geometry checks out. Before marking this PR ready, however, I suggest tightening both the problem definition and the E2E contract.

The current geometry is:

  • Project title: 8px row padding + 16px Folder icon + 8px gap = 32px
  • Session title before this PR: 24px child inset + 8px row padding + 8px StatusDot + 8px gap = 48px
  • Session title after this PR: 8px child inset + 8px row padding + 8px StatusDot + 8px gap = 32px

So the old layout does place session titles 16px too far to the right, and --spacing-2 aligns them exactly. The production change itself is minimal and appropriate.

I see three points worth addressing:

  1. The problem statement presents a product choice as a design-system fact. The claim that the 24px SideNav nesting step is specifically for icon-less children is not established by the current Astryx implementation; Astryx applies spacing-6 to children unconditionally. A more precise statement would be: “The parent and child rows have leading content of different widths. Combined with the fixed nesting inset, this misaligns their titles. The product intent is to align the titles while retaining an 8px hierarchical inset.” This matters because fix(desktop): indent project session rows #3175 only recently defined the full 24px nesting step as the visual contract. This PR is selecting a different contract, not merely correcting arithmetic. A before/after screenshot or design reference would make that intent explicit.

  2. The E2E assertion does not directly verify the stated result. It currently checks that the project and session button left edges differ by 6–16px, rather than checking that their title x-coordinates match. If the StatusDot width, icon size, or SideNav gap changes later, the titles could become misaligned while the test still passes. I suggest directly asserting abs(projectTitle.x - sessionTitle.x) <= 1–2px. If preserving hierarchy is also a contract, separately assert that the session button begins about 8px inside the project button.

  3. The visible effect is broader than the title. This change moves the left edge of the entire session button, including its hover/selected background and hit area, 16px to the left. The right edge should remain stable. That is likely acceptable, but it should be included in the behavior description.

From an Occam’s razor perspective, I would keep the current spacing-2 implementation. A calc() expression or a new component abstraction would add complexity without improving the result. The main improvement is to describe the actual product decision and test the visual outcome—title alignment—rather than pinning a CSS token or internal DOM mechanism.

AI-assisted review disclosure: This comment was prepared with Codex assistance after statically reviewing the PR diff, the Astryx SideNav/StatusDot layout, and the history of #3175.

Keep the 8px nest. State it as a product choice against SideNav's fixed
24px child inset, and check title x in the sidebar e2e instead of only
the session button edge.
Generated-by: Maka
@Astro-Han

Copy link
Copy Markdown
ContributorAuthor

Accepted points 1–3. Kept --spacing-2; that is still the smallest implementation.

  1. The 24px SideNav nest is a fixed child inset, not an icon-less-child alignment rule. This PR chooses a different product contract from fix(desktop): indent project session rows #3175: keep an 8px hierarchical inset so session titles share the project title's x. Comments in sidebar.css, the source contract, and the ProjectGroups story now say that.

  2. The sidebar e2e now asserts abs(projectTitle.x - sessionTitle.x) <= 2 and still checks that the session button starts about 8px inside the project button.

  3. The whole session button, including hover/selected fill and hit area, moves 16px left; the right edge stays on the rail. That is now in the CSS comment.

Before/after for the title column is on the desktop compare image from the earlier thread.

@Astro-Han
Astro-Han marked this pull request as ready for review August 20, 2026 14:56
@Astro-Han
Astro-Han requested a review from M4n5terAugust 20, 2026 14:56

@hqhq1025hqhq1025 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the exact current revision against the stated layout contract, production DOM/CSS path, prior maintainer feedback, and focused geometry/Electron coverage. No actionable defect survived the policy v7 materiality review.

Codex-assisted review performed under the maintainer-approved review workflow.

@Astro-Han
Astro-Han merged commit db0432e into mainAug 20, 2026
1 check passed
@Astro-Han
Astro-Han deleted the fix/sidebar-align-session-titles branch August 20, 2026 19:40
Sign up for freeto 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.

3 participants

@Astro-Han@likun666661@hqhq1025
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(desktop): align project session titles with the project name - #3311

Merged
Astro-Han merged 2 commits into
mainfrom
fix/sidebar-align-session-titles
Aug 20, 2026
Merged

fix(desktop): align project session titles with the project name#3311
Astro-Han merged 2 commits into
mainfrom
fix/sidebar-align-session-titles

Conversation

@Astro-Han

@Astro-HanAstro-Han commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Project-grouped session rows were nested a full Astryx SideNav step (24px). That step is for icon-less children so their text meets a parent title after a 16px icon + 8px gap. Session rows already spend 8px on StatusDot in that slot, so the extra 24px pushed titles too far right of the project name.

image

Keep the remaining 8px nest (--spacing-2) so titles share one x and the row still sits inside the project.

Verification

  • Ran node --experimental-strip-types --test apps/desktop/src/main/__tests__/session-project-hierarchy-contract.test.ts in the worktree (pass).
  • Did not run Playwright locally: the geometry spec opens a visible Electron window and would steal focus from an active maka dev session. Leave sidebar-project-row.spec.ts to CI.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope:

Maka diagnosed the indent, chose the 8px remaining nest, and authored the CSS, contract test, e2e assertion, and commit.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

Astryx nests icon-less children by 24px so their text meets a parent
title after a 16px icon + 8px gap. Session rows already spend 8px on
StatusDot in that slot, so keep the remaining 8px nest instead of a
full SideNav step.
Generated-by: Maka
@likun666661

likun666661 commented Aug 20, 2026

Copy link
Copy Markdown
Member

Overall, the direction is sound and the spacing-6 → spacing-2 geometry checks out. Before marking this PR ready, however, I suggest tightening both the problem definition and the E2E contract.

The current geometry is:

  • Project title: 8px row padding + 16px Folder icon + 8px gap = 32px
  • Session title before this PR: 24px child inset + 8px row padding + 8px StatusDot + 8px gap = 48px
  • Session title after this PR: 8px child inset + 8px row padding + 8px StatusDot + 8px gap = 32px

So the old layout does place session titles 16px too far to the right, and --spacing-2 aligns them exactly. The production change itself is minimal and appropriate.

I see three points worth addressing:

  1. The problem statement presents a product choice as a design-system fact. The claim that the 24px SideNav nesting step is specifically for icon-less children is not established by the current Astryx implementation; Astryx applies spacing-6 to children unconditionally. A more precise statement would be: “The parent and child rows have leading content of different widths. Combined with the fixed nesting inset, this misaligns their titles. The product intent is to align the titles while retaining an 8px hierarchical inset.” This matters because fix(desktop): indent project session rows #3175 only recently defined the full 24px nesting step as the visual contract. This PR is selecting a different contract, not merely correcting arithmetic. A before/after screenshot or design reference would make that intent explicit.

  2. The E2E assertion does not directly verify the stated result. It currently checks that the project and session button left edges differ by 6–16px, rather than checking that their title x-coordinates match. If the StatusDot width, icon size, or SideNav gap changes later, the titles could become misaligned while the test still passes. I suggest directly asserting abs(projectTitle.x - sessionTitle.x) <= 1–2px. If preserving hierarchy is also a contract, separately assert that the session button begins about 8px inside the project button.

  3. The visible effect is broader than the title. This change moves the left edge of the entire session button, including its hover/selected background and hit area, 16px to the left. The right edge should remain stable. That is likely acceptable, but it should be included in the behavior description.

From an Occam’s razor perspective, I would keep the current spacing-2 implementation. A calc() expression or a new component abstraction would add complexity without improving the result. The main improvement is to describe the actual product decision and test the visual outcome—title alignment—rather than pinning a CSS token or internal DOM mechanism.

AI-assisted review disclosure: This comment was prepared with Codex assistance after statically reviewing the PR diff, the Astryx SideNav/StatusDot layout, and the history of #3175.

Keep the 8px nest. State it as a product choice against SideNav's fixed
24px child inset, and check title x in the sidebar e2e instead of only
the session button edge.
Generated-by: Maka
@Astro-Han

Copy link
Copy Markdown
ContributorAuthor

Accepted points 1–3. Kept --spacing-2; that is still the smallest implementation.

  1. The 24px SideNav nest is a fixed child inset, not an icon-less-child alignment rule. This PR chooses a different product contract from fix(desktop): indent project session rows #3175: keep an 8px hierarchical inset so session titles share the project title's x. Comments in sidebar.css, the source contract, and the ProjectGroups story now say that.

  2. The sidebar e2e now asserts abs(projectTitle.x - sessionTitle.x) <= 2 and still checks that the session button starts about 8px inside the project button.

  3. The whole session button, including hover/selected fill and hit area, moves 16px left; the right edge stays on the rail. That is now in the CSS comment.

Before/after for the title column is on the desktop compare image from the earlier thread.

@Astro-Han
Astro-Han marked this pull request as ready for review August 20, 2026 14:56
@Astro-Han
Astro-Han requested a review from M4n5terAugust 20, 2026 14:56

@hqhq1025hqhq1025 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the exact current revision against the stated layout contract, production DOM/CSS path, prior maintainer feedback, and focused geometry/Electron coverage. No actionable defect survived the policy v7 materiality review.

Codex-assisted review performed under the maintainer-approved review workflow.

@Astro-Han
Astro-Han merged commit db0432e into mainAug 20, 2026
1 check passed
@Astro-Han
Astro-Han deleted the fix/sidebar-align-session-titles branch August 20, 2026 19:40
Sign up for freeto 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.

3 participants

@Astro-Han@likun666661@hqhq1025
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

fix(desktop): align project session titles with the project name - #3311

Merged
Astro-Han merged 2 commits into
mainfrom
fix/sidebar-align-session-titles
Aug 20, 2026
Merged

fix(desktop): align project session titles with the project name#3311
Astro-Han merged 2 commits into
mainfrom
fix/sidebar-align-session-titles

Conversation

@Astro-Han

@Astro-HanAstro-Han commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Project-grouped session rows were nested a full Astryx SideNav step (24px). That step is for icon-less children so their text meets a parent title after a 16px icon + 8px gap. Session rows already spend 8px on StatusDot in that slot, so the extra 24px pushed titles too far right of the project name.

image

Keep the remaining 8px nest (--spacing-2) so titles share one x and the row still sits inside the project.

Verification

  • Ran node --experimental-strip-types --test apps/desktop/src/main/__tests__/session-project-hierarchy-contract.test.ts in the worktree (pass).
  • Did not run Playwright locally: the geometry spec opens a visible Electron window and would steal focus from an active maka dev session. Leave sidebar-project-row.spec.ts to CI.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope:

Maka diagnosed the indent, chose the 8px remaining nest, and authored the CSS, contract test, e2e assertion, and commit.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

Astryx nests icon-less children by 24px so their text meets a parent
title after a 16px icon + 8px gap. Session rows already spend 8px on
StatusDot in that slot, so keep the remaining 8px nest instead of a
full SideNav step.
Generated-by: Maka
@likun666661

likun666661 commented Aug 20, 2026

Copy link
Copy Markdown
Member

Overall, the direction is sound and the spacing-6 → spacing-2 geometry checks out. Before marking this PR ready, however, I suggest tightening both the problem definition and the E2E contract.

The current geometry is:

  • Project title: 8px row padding + 16px Folder icon + 8px gap = 32px
  • Session title before this PR: 24px child inset + 8px row padding + 8px StatusDot + 8px gap = 48px
  • Session title after this PR: 8px child inset + 8px row padding + 8px StatusDot + 8px gap = 32px

So the old layout does place session titles 16px too far to the right, and --spacing-2 aligns them exactly. The production change itself is minimal and appropriate.

I see three points worth addressing:

  1. The problem statement presents a product choice as a design-system fact. The claim that the 24px SideNav nesting step is specifically for icon-less children is not established by the current Astryx implementation; Astryx applies spacing-6 to children unconditionally. A more precise statement would be: “The parent and child rows have leading content of different widths. Combined with the fixed nesting inset, this misaligns their titles. The product intent is to align the titles while retaining an 8px hierarchical inset.” This matters because fix(desktop): indent project session rows #3175 only recently defined the full 24px nesting step as the visual contract. This PR is selecting a different contract, not merely correcting arithmetic. A before/after screenshot or design reference would make that intent explicit.

  2. The E2E assertion does not directly verify the stated result. It currently checks that the project and session button left edges differ by 6–16px, rather than checking that their title x-coordinates match. If the StatusDot width, icon size, or SideNav gap changes later, the titles could become misaligned while the test still passes. I suggest directly asserting abs(projectTitle.x - sessionTitle.x) <= 1–2px. If preserving hierarchy is also a contract, separately assert that the session button begins about 8px inside the project button.

  3. The visible effect is broader than the title. This change moves the left edge of the entire session button, including its hover/selected background and hit area, 16px to the left. The right edge should remain stable. That is likely acceptable, but it should be included in the behavior description.

From an Occam’s razor perspective, I would keep the current spacing-2 implementation. A calc() expression or a new component abstraction would add complexity without improving the result. The main improvement is to describe the actual product decision and test the visual outcome—title alignment—rather than pinning a CSS token or internal DOM mechanism.

AI-assisted review disclosure: This comment was prepared with Codex assistance after statically reviewing the PR diff, the Astryx SideNav/StatusDot layout, and the history of #3175.

Keep the 8px nest. State it as a product choice against SideNav's fixed
24px child inset, and check title x in the sidebar e2e instead of only
the session button edge.
Generated-by: Maka
@Astro-Han

Copy link
Copy Markdown
ContributorAuthor

Accepted points 1–3. Kept --spacing-2; that is still the smallest implementation.

  1. The 24px SideNav nest is a fixed child inset, not an icon-less-child alignment rule. This PR chooses a different product contract from fix(desktop): indent project session rows #3175: keep an 8px hierarchical inset so session titles share the project title's x. Comments in sidebar.css, the source contract, and the ProjectGroups story now say that.

  2. The sidebar e2e now asserts abs(projectTitle.x - sessionTitle.x) <= 2 and still checks that the session button starts about 8px inside the project button.

  3. The whole session button, including hover/selected fill and hit area, moves 16px left; the right edge stays on the rail. That is now in the CSS comment.

Before/after for the title column is on the desktop compare image from the earlier thread.

@Astro-Han
Astro-Han marked this pull request as ready for review August 20, 2026 14:56
@Astro-Han
Astro-Han requested a review from M4n5terAugust 20, 2026 14:56

@hqhq1025hqhq1025 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the exact current revision against the stated layout contract, production DOM/CSS path, prior maintainer feedback, and focused geometry/Electron coverage. No actionable defect survived the policy v7 materiality review.

Codex-assisted review performed under the maintainer-approved review workflow.

@Astro-Han
Astro-Han merged commit db0432e into mainAug 20, 2026
1 check passed
@Astro-Han
Astro-Han deleted the fix/sidebar-align-session-titles branch August 20, 2026 19:40
Sign up for freeto 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.

3 participants

@Astro-Han@likun666661@hqhq1025