You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maka.apache.org by "1625567290 (via GitHub)" <gi...@apache.org> on 2026/08/22 06:17:55 UTC
[PR] fix(desktop): correct Agent Graph collapse chevron direction [maka]
1625567290 opened a new pull request, #3466:
URL: https://github.com/apache/maka/pull/3466
## Summary
- keep the Agent Graph chevron pointing down while the panel is expanded
- rotate the chevron only when aria-expanded is false so the collapsed state points up
- cover disclosure state, collapsed state, and controlled content synchronization
## Testing
- npm --workspace @maka/desktop run build:main
- node --test --test-name-pattern=AgentGraphPanel\ collapse apps/desktop/dist/main/__tests__/agent-graph-panel.test.js
- npm run build
- npm run lint
- npm run format:check
- npm run typecheck
- real Chrome computed-style verification for expanded and collapsed states
The concurrent Desktop suite reached 1059/1061. The unrelated Git Review and MCP runtime failures both passed when rerun independently.
Closes #3407.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] fix(desktop): correct Agent Graph collapse chevron direction [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han commented on code in PR #3466:
URL: https://github.com/apache/maka/pull/3466#discussion_r3835681825
##########
apps/desktop/src/main/__tests__/agent-graph-panel.test.ts:
##########
@@ -288,6 +288,37 @@ async function renderPanel(
return harness;
}
+describe('AgentGraphPanel collapse', () => {
+ it('keeps the disclosure state and controlled content in sync', async () => {
+ const harness = await renderPanel(snapshot({ graphId: 'graph-1', status: 'active' }));
+ const panel = harness.container.querySelector('.maka-agent-graph-panel');
+ const toggle = harness.container.querySelector('.maka-agent-graph-collapse-toggle');
+ assert.ok(panel);
+ assert.ok(toggle);
+ const contentId = toggle.getAttribute('aria-controls');
+ assert.ok(contentId);
+
+ assert.equal(toggle.getAttribute('aria-expanded'), 'true');
+ assert.equal(panel.getAttribute('data-collapsed'), 'false');
+ assert.ok(harness.container.querySelector(`#${contentId}`));
+
+ await act(async () => {
+ (toggle as HTMLElement).click();
+ });
+ assert.equal(toggle.getAttribute('aria-expanded'), 'false');
Review Comment:
[P3] This test pins the disclosure state and content, but not the actual chevron transform that regressed. A browser-level computed-style or small visual assertion would catch the selector drifting while all of these state assertions still pass. This is optional hardening and does not block this focused fix.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] fix(desktop): correct Agent Graph collapse chevron direction [maka]
Posted by "1625567290 (via GitHub)" <gi...@apache.org>.
1625567290 commented on code in PR #3466:
URL: https://github.com/apache/maka/pull/3466#discussion_r3837544820
##########
apps/desktop/e2e/agent-graph-chevron-smoke.mjs:
##########
@@ -0,0 +1,76 @@
+import { readFile } from 'node:fs/promises';
+import { app, BrowserWindow } from 'electron';
+
+async function main() {
+ const stylesheet = await readFile(
+ new URL('../src/renderer/styles/agent-graph.css', import.meta.url),
+ 'utf8',
+ );
+ await app.whenReady();
+ const window = new BrowserWindow({
+ show: false,
+ width: 320,
+ height: 200,
+ webPreferences: {
+ backgroundThrottling: false,
+ contextIsolation: true,
+ nodeIntegration: false,
+ sandbox: true,
+ },
+ });
Review Comment:
Agreed. Removed the isolated Electron smoke and its Playwright wrapper. The PR no longer launches a separate Electron process or asserts Chromium transform serialization; the final product diff is only the corrected CSS selector.
##########
apps/desktop/e2e/agent-graph-chevron.spec.ts:
##########
@@ -0,0 +1,15 @@
+import { execFile } from 'node:child_process';
+import { createRequire } from 'node:module';
+import { promisify } from 'node:util';
+import { test, expect } from '@playwright/test';
+
+const execFileAsync = promisify(execFile);
+const electronPath = createRequire(import.meta.url)('electron') as string;
+
+test('agent graph collapse chevron follows the disclosure state', async () => {
Review Comment:
Removed this spec and the child Electron process that caused the Linux SUID sandbox failure. The focused CSS fix no longer adds a real-window test to the critical E2E path.
##########
apps/desktop/src/main/__tests__/agent-graph-panel.test.ts:
##########
@@ -288,6 +288,37 @@ async function renderPanel(
return harness;
}
+describe('AgentGraphPanel collapse', () => {
+ it('keeps the disclosure state and controlled content in sync', async () => {
Review Comment:
Agreed. Removed the unrelated disclosure-state test as well. The final PR now contains only the one-line CSS selector correction, so the existing component behavior is no longer bundled into this change.
##########
apps/desktop/e2e/agent-graph-chevron-smoke.mjs:
##########
@@ -0,0 +1,76 @@
+import { readFile } from 'node:fs/promises';
+import { app, BrowserWindow } from 'electron';
+
+async function main() {
+ const stylesheet = await readFile(
+ new URL('../src/renderer/styles/agent-graph.css', import.meta.url),
+ 'utf8',
+ );
+ await app.whenReady();
+ const window = new BrowserWindow({
+ show: false,
+ width: 320,
+ height: 200,
+ webPreferences: {
+ backgroundThrottling: false,
+ contextIsolation: true,
+ nodeIntegration: false,
+ sandbox: true,
+ },
+ });
+ await window.loadURL(
+ `data:text/html;charset=utf-8,${encodeURIComponent(`<!doctype html>
+ <html>
+ <head>
+ <style>
+ :root { --duration-quick: 0s; --ease-out-strong: linear; }
+ ${stylesheet}
+ </style>
+ </head>
+ <body>
+ <button class="maka-agent-graph-collapse-toggle" aria-expanded="true">
+ <svg aria-hidden="true" viewBox="0 0 16 16"></svg>
+ </button>
+ </body>
+ </html>`)}`,
+ );
+
+ const transformFor = (expanded) =>
+ window.webContents.executeJavaScript(
+ `(async () => {
+ const toggle = document.querySelector('.maka-agent-graph-collapse-toggle');
+ toggle.setAttribute('aria-expanded', ${JSON.stringify(String(expanded))});
+ await new Promise((resolve) => requestAnimationFrame(() => resolve()));
+ return getComputedStyle(toggle.querySelector('svg')).transform;
+ })()`,
+ true,
+ );
+
+ try {
+ const expanded = await transformFor(true);
+ const collapsed = await transformFor(false);
+ const expandedAgain = await transformFor(true);
+ if (
+ expanded !== 'none' ||
+ collapsed !== 'matrix(-1, 0, 0, -1, 0, 0)' ||
Review Comment:
Removed with the rest of the isolated browser smoke. There is no longer an `expandedAgain` assertion or synthetic page in this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] fix(desktop): correct Agent Graph collapse chevron direction [maka]
Posted by "1625567290 (via GitHub)" <gi...@apache.org>.
1625567290 commented on PR #3466:
URL: https://github.com/apache/maka/pull/3466#issuecomment-5383942268
Update: the rebased head is mergeable and has no unresolved review threads. Its new CI run is currently `action_required`, so it needs maintainer approval before the replacement check can execute.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] fix(desktop): correct Agent Graph collapse chevron direction [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han commented on code in PR #3466:
URL: https://github.com/apache/maka/pull/3466#discussion_r3836029875
##########
apps/desktop/e2e/agent-graph-chevron.spec.ts:
##########
@@ -0,0 +1,15 @@
+import { execFile } from 'node:child_process';
+import { createRequire } from 'node:module';
+import { promisify } from 'node:util';
+import { test, expect } from '@playwright/test';
+
+const execFileAsync = promisify(execFile);
+const electronPath = createRequire(import.meta.url)('electron') as string;
+
+test('agent graph collapse chevron follows the disclosure state', async () => {
Review Comment:
`[P2]` CI is red on this head and this is what is failing:
```
test Desktop e2e Error: Command failed: .../node_modules/electron/dist/electron e2e/agent-graph-chevron-smoke.mjs
1 failed
```
A real `BrowserWindow` on a headless CI runner is a materially different environment from your machine, and `show: false` does not remove the dependency on a working display/GPU path. Whatever the specific cause, the situation is that a one-line CSS fix cannot land because of the test written to protect it.
Worth stating plainly since it cuts against the usual instinct: dropping this spec is a legitimate resolution here, not a retreat. The cost of the instrument should stay proportional to what it protects, and one CSS declaration does not justify an Electron process in the critical path of every CI run.
##########
apps/desktop/e2e/agent-graph-chevron-smoke.mjs:
##########
@@ -0,0 +1,76 @@
+import { readFile } from 'node:fs/promises';
+import { app, BrowserWindow } from 'electron';
+
+async function main() {
+ const stylesheet = await readFile(
+ new URL('../src/renderer/styles/agent-graph.css', import.meta.url),
+ 'utf8',
+ );
+ await app.whenReady();
+ const window = new BrowserWindow({
+ show: false,
+ width: 320,
+ height: 200,
+ webPreferences: {
+ backgroundThrottling: false,
+ contextIsolation: true,
+ nodeIntegration: false,
+ sandbox: true,
+ },
+ });
Review Comment:
`[P2]` This is the finding I care most about, and it holds independently of the CI failure.
The page under test is a hand-written `<button>` plus `agent-graph.css` alone, injected into a `data:` URL. Nothing else from the app is present. So this asserts that the rule works **when it is the only rule**.
But the regression this is guarding against does not look like that. A chevron points the wrong way because the selector is wrong (what you just fixed), because another stylesheet later wins the cascade, or because the component stops emitting `aria-expanded` on that element. Only the first is visible here — and the first is already visible far more cheaply. The second and third are exactly what an end-to-end test is supposed to buy, and this one is blind to both: it never loads the real stylesheet set and never renders the real component.
So the test costs a full Electron process per run and covers the one failure mode that did not need it.
Separately, `matrix(-1, 0, 0, -1, 0, 0)` pins Chromium's serialization of the transform rather than the intent. `rotate(-180deg)` is the same picture and fails this assertion.
##########
apps/desktop/e2e/agent-graph-chevron-smoke.mjs:
##########
@@ -0,0 +1,76 @@
+import { readFile } from 'node:fs/promises';
+import { app, BrowserWindow } from 'electron';
+
+async function main() {
+ const stylesheet = await readFile(
+ new URL('../src/renderer/styles/agent-graph.css', import.meta.url),
+ 'utf8',
+ );
+ await app.whenReady();
+ const window = new BrowserWindow({
+ show: false,
+ width: 320,
+ height: 200,
+ webPreferences: {
+ backgroundThrottling: false,
+ contextIsolation: true,
+ nodeIntegration: false,
+ sandbox: true,
+ },
+ });
+ await window.loadURL(
+ `data:text/html;charset=utf-8,${encodeURIComponent(`<!doctype html>
+ <html>
+ <head>
+ <style>
+ :root { --duration-quick: 0s; --ease-out-strong: linear; }
+ ${stylesheet}
+ </style>
+ </head>
+ <body>
+ <button class="maka-agent-graph-collapse-toggle" aria-expanded="true">
+ <svg aria-hidden="true" viewBox="0 0 16 16"></svg>
+ </button>
+ </body>
+ </html>`)}`,
+ );
+
+ const transformFor = (expanded) =>
+ window.webContents.executeJavaScript(
+ `(async () => {
+ const toggle = document.querySelector('.maka-agent-graph-collapse-toggle');
+ toggle.setAttribute('aria-expanded', ${JSON.stringify(String(expanded))});
+ await new Promise((resolve) => requestAnimationFrame(() => resolve()));
+ return getComputedStyle(toggle.querySelector('svg')).transform;
+ })()`,
+ true,
+ );
+
+ try {
+ const expanded = await transformFor(true);
+ const collapsed = await transformFor(false);
+ const expandedAgain = await transformFor(true);
+ if (
+ expanded !== 'none' ||
+ collapsed !== 'matrix(-1, 0, 0, -1, 0, 0)' ||
Review Comment:
`[P3]` `expandedAgain` cannot differ from `expanded`. Between the two reads the only thing that changed is the attribute, which is set explicitly each time, and there is no component, no state, and no transition (`--duration-quick` is pinned to `0s` above) that could carry anything across.
An assertion that cannot fail reads as coverage without being coverage.
##########
apps/desktop/src/main/__tests__/agent-graph-panel.test.ts:
##########
@@ -288,6 +288,37 @@ async function renderPanel(
return harness;
}
+describe('AgentGraphPanel collapse', () => {
+ it('keeps the disclosure state and controlled content in sync', async () => {
Review Comment:
`[P3]` This test is well written — it drives the real component through the existing harness and asserts `aria-expanded`, `data-collapsed`, and that the `aria-controls` target actually mounts and unmounts. If I had to keep exactly one of the three tests in this PR, it would be this one.
The note is only that none of the behaviour it covers is changed by this PR. Disclosure-state syncing worked before and works after; the diff to the product is one CSS selector. So a reviewer reading `+122/-1` has to work out that 121 of those lines are unrelated coverage.
Not asking you to remove it — new coverage of untested behaviour is worth having. Just noting that it belongs in its own change, where it can be judged on its own merits rather than riding along with a chevron fix.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] fix(desktop): correct Agent Graph collapse chevron direction [maka]
Posted by "1625567290 (via GitHub)" <gi...@apache.org>.
1625567290 commented on code in PR #3466:
URL: https://github.com/apache/maka/pull/3466#discussion_r3835863235
##########
apps/desktop/src/main/__tests__/agent-graph-panel.test.ts:
##########
@@ -288,6 +288,37 @@ async function renderPanel(
return harness;
}
+describe('AgentGraphPanel collapse', () => {
+ it('keeps the disclosure state and controlled content in sync', async () => {
+ const harness = await renderPanel(snapshot({ graphId: 'graph-1', status: 'active' }));
+ const panel = harness.container.querySelector('.maka-agent-graph-panel');
+ const toggle = harness.container.querySelector('.maka-agent-graph-collapse-toggle');
+ assert.ok(panel);
+ assert.ok(toggle);
+ const contentId = toggle.getAttribute('aria-controls');
+ assert.ok(contentId);
+
+ assert.equal(toggle.getAttribute('aria-expanded'), 'true');
+ assert.equal(panel.getAttribute('data-collapsed'), 'false');
+ assert.ok(harness.container.querySelector(`#${contentId}`));
+
+ await act(async () => {
+ (toggle as HTMLElement).click();
+ });
+ assert.equal(toggle.getAttribute('aria-expanded'), 'false');
Review Comment:
Added a real Electron/Chromium computed-style smoke in , backed by . It loads the production and verifies expanded , collapsed , then the restored expanded state. Local browser smoke passed 1/1, the panel suite passed 15/15, desktop typecheck passed, and the full branch CI passed: https://github.com/1625567290/maka-agent/actions/runs/32568735415
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] fix(desktop): correct Agent Graph collapse chevron direction [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han merged PR #3466:
URL: https://github.com/apache/maka/pull/3466
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] fix(desktop): correct Agent Graph collapse chevron direction [maka]
Posted by "1625567290 (via GitHub)" <gi...@apache.org>.
1625567290 commented on PR #3466:
URL: https://github.com/apache/maka/pull/3466#issuecomment-5383833932
Rebased onto current `main` and reduced the PR to the one-line CSS selector correction. The isolated Electron smoke, its Playwright wrapper, and the unrelated disclosure-state test have been removed, addressing the four review threads and eliminating the Linux SUID sandbox failure path.
Local validation: Desktop workspace dependencies and main process build passed, the existing Agent Graph panel suite passed 14/14 before the unrelated added case was removed, Desktop typecheck passed, and lint, format, ASF header, and diff checks are clean. New CI is now pending on head `608251142`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] fix(desktop): correct Agent Graph collapse chevron direction [maka]
Posted by "1625567290 (via GitHub)" <gi...@apache.org>.
1625567290 commented on code in PR #3466:
URL: https://github.com/apache/maka/pull/3466#discussion_r3835863235
##########
apps/desktop/src/main/__tests__/agent-graph-panel.test.ts:
##########
@@ -288,6 +288,37 @@ async function renderPanel(
return harness;
}
+describe('AgentGraphPanel collapse', () => {
+ it('keeps the disclosure state and controlled content in sync', async () => {
+ const harness = await renderPanel(snapshot({ graphId: 'graph-1', status: 'active' }));
+ const panel = harness.container.querySelector('.maka-agent-graph-panel');
+ const toggle = harness.container.querySelector('.maka-agent-graph-collapse-toggle');
+ assert.ok(panel);
+ assert.ok(toggle);
+ const contentId = toggle.getAttribute('aria-controls');
+ assert.ok(contentId);
+
+ assert.equal(toggle.getAttribute('aria-expanded'), 'true');
+ assert.equal(panel.getAttribute('data-collapsed'), 'false');
+ assert.ok(harness.container.querySelector(`#${contentId}`));
+
+ await act(async () => {
+ (toggle as HTMLElement).click();
+ });
+ assert.equal(toggle.getAttribute('aria-expanded'), 'false');
Review Comment:
Added a real Electron/Chromium computed-style smoke in apps/desktop/e2e/agent-graph-chevron.spec.ts, backed by apps/desktop/e2e/agent-graph-chevron-smoke.mjs. It loads the production CSS and verifies the expanded transform is none, the collapsed transform is matrix(-1, 0, 0, -1, 0, 0), then restores the expanded state. Local browser smoke passed 1/1, the panel suite passed 15/15, desktop typecheck passed, and the full branch CI passed: https://github.com/1625567290/maka-agent/actions/runs/32568735415
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org