refactor(ui): simplify the Skills page on Astryx - #1973

Merged
Astro-Han merged 3 commits into
apache:mainfrom
me2seeks:fix/1901-skills-astryx-redesign
Aug 4, 2026
Merged

refactor(ui): simplify the Skills page on Astryx#1973
Astro-Han merged 3 commits into
apache:mainfrom
me2seeks:fix/1901-skills-astryx-redesign

Conversation

@me2seeks

@me2seeksme2seeks commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Summary

  • replace the promotional/card stack and hand-built Skill rows with Astryx Item surfaces
  • keep one direct Use action and one state switch per installed Skill, with open, pin, update, and delete in a single contextual menu
  • keep source and scope as inline supporting metadata, reduce skills.css from 618 to 258 lines, and add the requested Storybook states plus UI/Desktop coverage

Rationale

The page gave promotional content, discovery, context diagnostics, and every row action similar visual weight. This makes discovery and installed management easier to scan while preserving source import, installation, update review, enablement, pinning, deletion, file opening, and explicit invocation.

This is rebased onto current main and does not restore the removed sample-Skill action. The remaining page-level open-folder and refresh actions move into one secondary menu.

Verification

  • npm --workspace @maka/ui test — 321 passed
  • npm --workspace @maka/desktop run typecheck:stories
  • npm --workspace @maka/desktop run build-storybook
  • npm --workspace @maka/desktop run smoke:storybook — 68 manifest checks and 78 catalog renders passed
  • npm --workspace @maka/desktop run test:checks
  • workspace dependency build across core, storage, MCP, runtime, runtime host, computer use, and UI
  • Storybook annotation and visual-smoke harness tests — 9 passed
  • targeted Playwright collection — 3 tests across the Skills hierarchy and scope-aware deletion journeys
  • committed update-review Storybook journey opens the row menu and diff panel, applies the source update, and asserts the exact current/source SHA arguments

The committed Playwright specs use normal pointer interactions. The update-review journey also runs through the real Astryx menu layer in the Storybook browser smoke rather than asserting static menu text only.

Closes#1901

中文说明
  • 使用 Astryx Item 重组市场、内置和已安装 Skill 列表,移除促销区与重复的卡片框架。
  • 每个已安装 Skill 只保留一个直接“使用”动作和一个启停开关;打开、固定、更新、删除收进同一个更多菜单。
  • 来源与作用域改为行内辅助信息,skills.css 从 618 行缩减到 258 行,并补齐空列表、已安装、内置、可更新、已停用和窄窗口 Storybook 场景及相关测试。
  • 新增可更新场景的真实交互测试:打开行菜单与差异面板、应用来源版本,并校验传入更新回调的 current/source SHA。
  • 本分支已重放到最新主线,不会重新引入已删除的“创建示例 Skill”入口。

@me2seeks

me2seeks commented Aug 3, 2026

Copy link
Copy Markdown
ContributorAuthor

The Skills-specific checks are passing. Storybook covered the installed view at wide, compact, and floor sizes, along with all five new catalog states. All three Skills E2E tests also passed in shard 2.

The red checks come from the Settings changes in #1972: Storybook fails on daily-review-model-selector-open-narrow, and shard 2 fails only on e2e/settings.spec.ts while focusing the theme radio group. #1972 has the same failures in its own CI run, and the aggregate e2e check simply reflects the shard result.

@Astro-HanAstro-Han 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.

Review: MERGE-READY ✅ (2 independent deepseek-v4-flash passes)

Reviewed the full diff (+564/−703, 9 files), ran the UI suite, and verified the Astryx contracts.

  • Feature parity preserved: install (market/built-in), import sources, enable/disable, direct Use, open SKILL.md, pin, update review, delete (now modal-confirm — safer than the old two-stage timer), refresh, open directory, context check, search/filter/sort, empty state, slug-collision display, exception badge, actionBusy guard — all retained; only the promo banner (removed per #1901) and row hover background were dropped.
  • Astryx className pitfall checked and clear: Item/DropdownMenuItem append consumer className/style via mergePropsafter internal stylex classes (no replace-semantics trap, unlike the pattern that broke #1738's search input).
  • Deletion is clean: the 3 removed CSS tokens and 7 removed copy keys have zero remaining consumers repo-wide (verified by grep); new CSS classes all have consumers; check-story-annotations passes.
  • Tests verified: new skills-panel.test.tsx (4 cases) passes; whole @maka/ui suite 272/272 green; rewritten skill-delete-scope.spec.ts + new skills.spec.ts aria/role assertions match Astryx's real DOM contracts. No "lying" old tests.
  • State discipline improved: timer-based delete-confirm effect (which violated the repo's "Effects only for external sync" convention) removed in favor of event-handler-driven dialog flow.

P2 to handle or explicitly defer: the update-review render journey (menu item → diff panel → apply) was rewritten but has zero committed test coverage (existing tests stop at menuitem existence; no story opens the UPDATE_AVAILABLE_PREVIEW fixture; PR body notes only manual exercise). Recommend adding one render test asserting the review panel opens and "update to source" calls onUpdateManagedSkill with the right sha, or record an explicit deferral.

Non-blocking P3: duplicate rAF-delay helper (skills-panel.tsx:112 vs :781); no in-progress indicator during delete; hover tooltip narrowed to label; hidden .maka-skill-tool-summary-hidden span retained.

Merge prerequisites: (1) the branch is ~100 commits behind main (merge-base 8e5008e vs main 1caea26; GitHub reports MERGEABLE) — CI evidence ran against the old base, so please rebase onto latest main and re-run CI; (2) the existing reds on the old base (settings theme focus flake, daily-review storybook contract) are unrelated to this PR and should disappear on a fresh run.

@me2seeks
me2seeksforce-pushed the fix/1901-skills-astryx-redesign branch from 0bf7b71 to 26dc5a8CompareAugust 4, 2026 13:40
@me2seeks

Copy link
Copy Markdown
ContributorAuthor

Rebased this onto current main (bc12cddf) and added the missing update-review render journey in 26dc5a81. The Storybook play now opens the row menu and diff panel, applies the source update, and asserts the exact current/source SHA arguments. All fresh checks are green (9 passed, 2 skipped by path). @Astro-Han, when you have a moment, could you take another look?

@Astro-Han

Copy link
Copy Markdown
Contributor

LGTM! merging.

@Astro-Han
Astro-Han merged commit 16f1470 into apache:mainAug 4, 2026
11 checks passed
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.

fix(ui): redesign the Skills page on Astryx primitives

2 participants

@me2seeks@Astro-Han
, '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

refactor(ui): simplify the Skills page on Astryx - #1973

Merged
Astro-Han merged 3 commits into
apache:mainfrom
me2seeks:fix/1901-skills-astryx-redesign
Aug 4, 2026
Merged

refactor(ui): simplify the Skills page on Astryx#1973
Astro-Han merged 3 commits into
apache:mainfrom
me2seeks:fix/1901-skills-astryx-redesign

Conversation

@me2seeks

@me2seeksme2seeks commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Summary

  • replace the promotional/card stack and hand-built Skill rows with Astryx Item surfaces
  • keep one direct Use action and one state switch per installed Skill, with open, pin, update, and delete in a single contextual menu
  • keep source and scope as inline supporting metadata, reduce skills.css from 618 to 258 lines, and add the requested Storybook states plus UI/Desktop coverage

Rationale

The page gave promotional content, discovery, context diagnostics, and every row action similar visual weight. This makes discovery and installed management easier to scan while preserving source import, installation, update review, enablement, pinning, deletion, file opening, and explicit invocation.

This is rebased onto current main and does not restore the removed sample-Skill action. The remaining page-level open-folder and refresh actions move into one secondary menu.

Verification

  • npm --workspace @maka/ui test — 321 passed
  • npm --workspace @maka/desktop run typecheck:stories
  • npm --workspace @maka/desktop run build-storybook
  • npm --workspace @maka/desktop run smoke:storybook — 68 manifest checks and 78 catalog renders passed
  • npm --workspace @maka/desktop run test:checks
  • workspace dependency build across core, storage, MCP, runtime, runtime host, computer use, and UI
  • Storybook annotation and visual-smoke harness tests — 9 passed
  • targeted Playwright collection — 3 tests across the Skills hierarchy and scope-aware deletion journeys
  • committed update-review Storybook journey opens the row menu and diff panel, applies the source update, and asserts the exact current/source SHA arguments

The committed Playwright specs use normal pointer interactions. The update-review journey also runs through the real Astryx menu layer in the Storybook browser smoke rather than asserting static menu text only.

Closes#1901

中文说明
  • 使用 Astryx Item 重组市场、内置和已安装 Skill 列表,移除促销区与重复的卡片框架。
  • 每个已安装 Skill 只保留一个直接“使用”动作和一个启停开关;打开、固定、更新、删除收进同一个更多菜单。
  • 来源与作用域改为行内辅助信息,skills.css 从 618 行缩减到 258 行,并补齐空列表、已安装、内置、可更新、已停用和窄窗口 Storybook 场景及相关测试。
  • 新增可更新场景的真实交互测试:打开行菜单与差异面板、应用来源版本,并校验传入更新回调的 current/source SHA。
  • 本分支已重放到最新主线,不会重新引入已删除的“创建示例 Skill”入口。

@me2seeks

me2seeks commented Aug 3, 2026

Copy link
Copy Markdown
ContributorAuthor

The Skills-specific checks are passing. Storybook covered the installed view at wide, compact, and floor sizes, along with all five new catalog states. All three Skills E2E tests also passed in shard 2.

The red checks come from the Settings changes in #1972: Storybook fails on daily-review-model-selector-open-narrow, and shard 2 fails only on e2e/settings.spec.ts while focusing the theme radio group. #1972 has the same failures in its own CI run, and the aggregate e2e check simply reflects the shard result.

@Astro-HanAstro-Han 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.

Review: MERGE-READY ✅ (2 independent deepseek-v4-flash passes)

Reviewed the full diff (+564/−703, 9 files), ran the UI suite, and verified the Astryx contracts.

  • Feature parity preserved: install (market/built-in), import sources, enable/disable, direct Use, open SKILL.md, pin, update review, delete (now modal-confirm — safer than the old two-stage timer), refresh, open directory, context check, search/filter/sort, empty state, slug-collision display, exception badge, actionBusy guard — all retained; only the promo banner (removed per #1901) and row hover background were dropped.
  • Astryx className pitfall checked and clear: Item/DropdownMenuItem append consumer className/style via mergePropsafter internal stylex classes (no replace-semantics trap, unlike the pattern that broke #1738's search input).
  • Deletion is clean: the 3 removed CSS tokens and 7 removed copy keys have zero remaining consumers repo-wide (verified by grep); new CSS classes all have consumers; check-story-annotations passes.
  • Tests verified: new skills-panel.test.tsx (4 cases) passes; whole @maka/ui suite 272/272 green; rewritten skill-delete-scope.spec.ts + new skills.spec.ts aria/role assertions match Astryx's real DOM contracts. No "lying" old tests.
  • State discipline improved: timer-based delete-confirm effect (which violated the repo's "Effects only for external sync" convention) removed in favor of event-handler-driven dialog flow.

P2 to handle or explicitly defer: the update-review render journey (menu item → diff panel → apply) was rewritten but has zero committed test coverage (existing tests stop at menuitem existence; no story opens the UPDATE_AVAILABLE_PREVIEW fixture; PR body notes only manual exercise). Recommend adding one render test asserting the review panel opens and "update to source" calls onUpdateManagedSkill with the right sha, or record an explicit deferral.

Non-blocking P3: duplicate rAF-delay helper (skills-panel.tsx:112 vs :781); no in-progress indicator during delete; hover tooltip narrowed to label; hidden .maka-skill-tool-summary-hidden span retained.

Merge prerequisites: (1) the branch is ~100 commits behind main (merge-base 8e5008e vs main 1caea26; GitHub reports MERGEABLE) — CI evidence ran against the old base, so please rebase onto latest main and re-run CI; (2) the existing reds on the old base (settings theme focus flake, daily-review storybook contract) are unrelated to this PR and should disappear on a fresh run.

@me2seeks
me2seeksforce-pushed the fix/1901-skills-astryx-redesign branch from 0bf7b71 to 26dc5a8CompareAugust 4, 2026 13:40
@me2seeks

Copy link
Copy Markdown
ContributorAuthor

Rebased this onto current main (bc12cddf) and added the missing update-review render journey in 26dc5a81. The Storybook play now opens the row menu and diff panel, applies the source update, and asserts the exact current/source SHA arguments. All fresh checks are green (9 passed, 2 skipped by path). @Astro-Han, when you have a moment, could you take another look?

@Astro-Han

Copy link
Copy Markdown
Contributor

LGTM! merging.

@Astro-Han
Astro-Han merged commit 16f1470 into apache:mainAug 4, 2026
11 checks passed
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.

fix(ui): redesign the Skills page on Astryx primitives

2 participants

@me2seeks@Astro-Han
, '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

refactor(ui): simplify the Skills page on Astryx - #1973

Merged
Astro-Han merged 3 commits into
apache:mainfrom
me2seeks:fix/1901-skills-astryx-redesign
Aug 4, 2026
Merged

refactor(ui): simplify the Skills page on Astryx#1973
Astro-Han merged 3 commits into
apache:mainfrom
me2seeks:fix/1901-skills-astryx-redesign

Conversation

@me2seeks

@me2seeksme2seeks commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Summary

  • replace the promotional/card stack and hand-built Skill rows with Astryx Item surfaces
  • keep one direct Use action and one state switch per installed Skill, with open, pin, update, and delete in a single contextual menu
  • keep source and scope as inline supporting metadata, reduce skills.css from 618 to 258 lines, and add the requested Storybook states plus UI/Desktop coverage

Rationale

The page gave promotional content, discovery, context diagnostics, and every row action similar visual weight. This makes discovery and installed management easier to scan while preserving source import, installation, update review, enablement, pinning, deletion, file opening, and explicit invocation.

This is rebased onto current main and does not restore the removed sample-Skill action. The remaining page-level open-folder and refresh actions move into one secondary menu.

Verification

  • npm --workspace @maka/ui test — 321 passed
  • npm --workspace @maka/desktop run typecheck:stories
  • npm --workspace @maka/desktop run build-storybook
  • npm --workspace @maka/desktop run smoke:storybook — 68 manifest checks and 78 catalog renders passed
  • npm --workspace @maka/desktop run test:checks
  • workspace dependency build across core, storage, MCP, runtime, runtime host, computer use, and UI
  • Storybook annotation and visual-smoke harness tests — 9 passed
  • targeted Playwright collection — 3 tests across the Skills hierarchy and scope-aware deletion journeys
  • committed update-review Storybook journey opens the row menu and diff panel, applies the source update, and asserts the exact current/source SHA arguments

The committed Playwright specs use normal pointer interactions. The update-review journey also runs through the real Astryx menu layer in the Storybook browser smoke rather than asserting static menu text only.

Closes#1901

中文说明
  • 使用 Astryx Item 重组市场、内置和已安装 Skill 列表,移除促销区与重复的卡片框架。
  • 每个已安装 Skill 只保留一个直接“使用”动作和一个启停开关;打开、固定、更新、删除收进同一个更多菜单。
  • 来源与作用域改为行内辅助信息,skills.css 从 618 行缩减到 258 行,并补齐空列表、已安装、内置、可更新、已停用和窄窗口 Storybook 场景及相关测试。
  • 新增可更新场景的真实交互测试:打开行菜单与差异面板、应用来源版本,并校验传入更新回调的 current/source SHA。
  • 本分支已重放到最新主线,不会重新引入已删除的“创建示例 Skill”入口。

@me2seeks

me2seeks commented Aug 3, 2026

Copy link
Copy Markdown
ContributorAuthor

The Skills-specific checks are passing. Storybook covered the installed view at wide, compact, and floor sizes, along with all five new catalog states. All three Skills E2E tests also passed in shard 2.

The red checks come from the Settings changes in #1972: Storybook fails on daily-review-model-selector-open-narrow, and shard 2 fails only on e2e/settings.spec.ts while focusing the theme radio group. #1972 has the same failures in its own CI run, and the aggregate e2e check simply reflects the shard result.

@Astro-HanAstro-Han 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.

Review: MERGE-READY ✅ (2 independent deepseek-v4-flash passes)

Reviewed the full diff (+564/−703, 9 files), ran the UI suite, and verified the Astryx contracts.

  • Feature parity preserved: install (market/built-in), import sources, enable/disable, direct Use, open SKILL.md, pin, update review, delete (now modal-confirm — safer than the old two-stage timer), refresh, open directory, context check, search/filter/sort, empty state, slug-collision display, exception badge, actionBusy guard — all retained; only the promo banner (removed per #1901) and row hover background were dropped.
  • Astryx className pitfall checked and clear: Item/DropdownMenuItem append consumer className/style via mergePropsafter internal stylex classes (no replace-semantics trap, unlike the pattern that broke #1738's search input).
  • Deletion is clean: the 3 removed CSS tokens and 7 removed copy keys have zero remaining consumers repo-wide (verified by grep); new CSS classes all have consumers; check-story-annotations passes.
  • Tests verified: new skills-panel.test.tsx (4 cases) passes; whole @maka/ui suite 272/272 green; rewritten skill-delete-scope.spec.ts + new skills.spec.ts aria/role assertions match Astryx's real DOM contracts. No "lying" old tests.
  • State discipline improved: timer-based delete-confirm effect (which violated the repo's "Effects only for external sync" convention) removed in favor of event-handler-driven dialog flow.

P2 to handle or explicitly defer: the update-review render journey (menu item → diff panel → apply) was rewritten but has zero committed test coverage (existing tests stop at menuitem existence; no story opens the UPDATE_AVAILABLE_PREVIEW fixture; PR body notes only manual exercise). Recommend adding one render test asserting the review panel opens and "update to source" calls onUpdateManagedSkill with the right sha, or record an explicit deferral.

Non-blocking P3: duplicate rAF-delay helper (skills-panel.tsx:112 vs :781); no in-progress indicator during delete; hover tooltip narrowed to label; hidden .maka-skill-tool-summary-hidden span retained.

Merge prerequisites: (1) the branch is ~100 commits behind main (merge-base 8e5008e vs main 1caea26; GitHub reports MERGEABLE) — CI evidence ran against the old base, so please rebase onto latest main and re-run CI; (2) the existing reds on the old base (settings theme focus flake, daily-review storybook contract) are unrelated to this PR and should disappear on a fresh run.

@me2seeks
me2seeksforce-pushed the fix/1901-skills-astryx-redesign branch from 0bf7b71 to 26dc5a8CompareAugust 4, 2026 13:40
@me2seeks

Copy link
Copy Markdown
ContributorAuthor

Rebased this onto current main (bc12cddf) and added the missing update-review render journey in 26dc5a81. The Storybook play now opens the row menu and diff panel, applies the source update, and asserts the exact current/source SHA arguments. All fresh checks are green (9 passed, 2 skipped by path). @Astro-Han, when you have a moment, could you take another look?

@Astro-Han

Copy link
Copy Markdown
Contributor

LGTM! merging.

@Astro-Han
Astro-Han merged commit 16f1470 into apache:mainAug 4, 2026
11 checks passed
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.

fix(ui): redesign the Skills page on Astryx primitives

2 participants

@me2seeks@Astro-Han
, '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

refactor(ui): simplify the Skills page on Astryx - #1973

Merged
Astro-Han merged 3 commits into
apache:mainfrom
me2seeks:fix/1901-skills-astryx-redesign
Aug 4, 2026
Merged

refactor(ui): simplify the Skills page on Astryx#1973
Astro-Han merged 3 commits into
apache:mainfrom
me2seeks:fix/1901-skills-astryx-redesign

Conversation

@me2seeks

@me2seeksme2seeks commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Summary

  • replace the promotional/card stack and hand-built Skill rows with Astryx Item surfaces
  • keep one direct Use action and one state switch per installed Skill, with open, pin, update, and delete in a single contextual menu
  • keep source and scope as inline supporting metadata, reduce skills.css from 618 to 258 lines, and add the requested Storybook states plus UI/Desktop coverage

Rationale

The page gave promotional content, discovery, context diagnostics, and every row action similar visual weight. This makes discovery and installed management easier to scan while preserving source import, installation, update review, enablement, pinning, deletion, file opening, and explicit invocation.

This is rebased onto current main and does not restore the removed sample-Skill action. The remaining page-level open-folder and refresh actions move into one secondary menu.

Verification

  • npm --workspace @maka/ui test — 321 passed
  • npm --workspace @maka/desktop run typecheck:stories
  • npm --workspace @maka/desktop run build-storybook
  • npm --workspace @maka/desktop run smoke:storybook — 68 manifest checks and 78 catalog renders passed
  • npm --workspace @maka/desktop run test:checks
  • workspace dependency build across core, storage, MCP, runtime, runtime host, computer use, and UI
  • Storybook annotation and visual-smoke harness tests — 9 passed
  • targeted Playwright collection — 3 tests across the Skills hierarchy and scope-aware deletion journeys
  • committed update-review Storybook journey opens the row menu and diff panel, applies the source update, and asserts the exact current/source SHA arguments

The committed Playwright specs use normal pointer interactions. The update-review journey also runs through the real Astryx menu layer in the Storybook browser smoke rather than asserting static menu text only.

Closes#1901

中文说明
  • 使用 Astryx Item 重组市场、内置和已安装 Skill 列表,移除促销区与重复的卡片框架。
  • 每个已安装 Skill 只保留一个直接“使用”动作和一个启停开关;打开、固定、更新、删除收进同一个更多菜单。
  • 来源与作用域改为行内辅助信息,skills.css 从 618 行缩减到 258 行,并补齐空列表、已安装、内置、可更新、已停用和窄窗口 Storybook 场景及相关测试。
  • 新增可更新场景的真实交互测试:打开行菜单与差异面板、应用来源版本,并校验传入更新回调的 current/source SHA。
  • 本分支已重放到最新主线,不会重新引入已删除的“创建示例 Skill”入口。

@me2seeks

me2seeks commented Aug 3, 2026

Copy link
Copy Markdown
ContributorAuthor

The Skills-specific checks are passing. Storybook covered the installed view at wide, compact, and floor sizes, along with all five new catalog states. All three Skills E2E tests also passed in shard 2.

The red checks come from the Settings changes in #1972: Storybook fails on daily-review-model-selector-open-narrow, and shard 2 fails only on e2e/settings.spec.ts while focusing the theme radio group. #1972 has the same failures in its own CI run, and the aggregate e2e check simply reflects the shard result.

@Astro-HanAstro-Han 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.

Review: MERGE-READY ✅ (2 independent deepseek-v4-flash passes)

Reviewed the full diff (+564/−703, 9 files), ran the UI suite, and verified the Astryx contracts.

  • Feature parity preserved: install (market/built-in), import sources, enable/disable, direct Use, open SKILL.md, pin, update review, delete (now modal-confirm — safer than the old two-stage timer), refresh, open directory, context check, search/filter/sort, empty state, slug-collision display, exception badge, actionBusy guard — all retained; only the promo banner (removed per #1901) and row hover background were dropped.
  • Astryx className pitfall checked and clear: Item/DropdownMenuItem append consumer className/style via mergePropsafter internal stylex classes (no replace-semantics trap, unlike the pattern that broke #1738's search input).
  • Deletion is clean: the 3 removed CSS tokens and 7 removed copy keys have zero remaining consumers repo-wide (verified by grep); new CSS classes all have consumers; check-story-annotations passes.
  • Tests verified: new skills-panel.test.tsx (4 cases) passes; whole @maka/ui suite 272/272 green; rewritten skill-delete-scope.spec.ts + new skills.spec.ts aria/role assertions match Astryx's real DOM contracts. No "lying" old tests.
  • State discipline improved: timer-based delete-confirm effect (which violated the repo's "Effects only for external sync" convention) removed in favor of event-handler-driven dialog flow.

P2 to handle or explicitly defer: the update-review render journey (menu item → diff panel → apply) was rewritten but has zero committed test coverage (existing tests stop at menuitem existence; no story opens the UPDATE_AVAILABLE_PREVIEW fixture; PR body notes only manual exercise). Recommend adding one render test asserting the review panel opens and "update to source" calls onUpdateManagedSkill with the right sha, or record an explicit deferral.

Non-blocking P3: duplicate rAF-delay helper (skills-panel.tsx:112 vs :781); no in-progress indicator during delete; hover tooltip narrowed to label; hidden .maka-skill-tool-summary-hidden span retained.

Merge prerequisites: (1) the branch is ~100 commits behind main (merge-base 8e5008e vs main 1caea26; GitHub reports MERGEABLE) — CI evidence ran against the old base, so please rebase onto latest main and re-run CI; (2) the existing reds on the old base (settings theme focus flake, daily-review storybook contract) are unrelated to this PR and should disappear on a fresh run.

@me2seeks
me2seeksforce-pushed the fix/1901-skills-astryx-redesign branch from 0bf7b71 to 26dc5a8CompareAugust 4, 2026 13:40
@me2seeks

Copy link
Copy Markdown
ContributorAuthor

Rebased this onto current main (bc12cddf) and added the missing update-review render journey in 26dc5a81. The Storybook play now opens the row menu and diff panel, applies the source update, and asserts the exact current/source SHA arguments. All fresh checks are green (9 passed, 2 skipped by path). @Astro-Han, when you have a moment, could you take another look?

@Astro-Han

Copy link
Copy Markdown
Contributor

LGTM! merging.

@Astro-Han
Astro-Han merged commit 16f1470 into apache:mainAug 4, 2026
11 checks passed
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.

fix(ui): redesign the Skills page on Astryx primitives

2 participants

@me2seeks@Astro-Han
, '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

refactor(ui): simplify the Skills page on Astryx - #1973

Merged
Astro-Han merged 3 commits into
apache:mainfrom
me2seeks:fix/1901-skills-astryx-redesign
Aug 4, 2026
Merged

refactor(ui): simplify the Skills page on Astryx#1973
Astro-Han merged 3 commits into
apache:mainfrom
me2seeks:fix/1901-skills-astryx-redesign

Conversation

@me2seeks

@me2seeksme2seeks commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Summary

  • replace the promotional/card stack and hand-built Skill rows with Astryx Item surfaces
  • keep one direct Use action and one state switch per installed Skill, with open, pin, update, and delete in a single contextual menu
  • keep source and scope as inline supporting metadata, reduce skills.css from 618 to 258 lines, and add the requested Storybook states plus UI/Desktop coverage

Rationale

The page gave promotional content, discovery, context diagnostics, and every row action similar visual weight. This makes discovery and installed management easier to scan while preserving source import, installation, update review, enablement, pinning, deletion, file opening, and explicit invocation.

This is rebased onto current main and does not restore the removed sample-Skill action. The remaining page-level open-folder and refresh actions move into one secondary menu.

Verification

  • npm --workspace @maka/ui test — 321 passed
  • npm --workspace @maka/desktop run typecheck:stories
  • npm --workspace @maka/desktop run build-storybook
  • npm --workspace @maka/desktop run smoke:storybook — 68 manifest checks and 78 catalog renders passed
  • npm --workspace @maka/desktop run test:checks
  • workspace dependency build across core, storage, MCP, runtime, runtime host, computer use, and UI
  • Storybook annotation and visual-smoke harness tests — 9 passed
  • targeted Playwright collection — 3 tests across the Skills hierarchy and scope-aware deletion journeys
  • committed update-review Storybook journey opens the row menu and diff panel, applies the source update, and asserts the exact current/source SHA arguments

The committed Playwright specs use normal pointer interactions. The update-review journey also runs through the real Astryx menu layer in the Storybook browser smoke rather than asserting static menu text only.

Closes#1901

中文说明
  • 使用 Astryx Item 重组市场、内置和已安装 Skill 列表,移除促销区与重复的卡片框架。
  • 每个已安装 Skill 只保留一个直接“使用”动作和一个启停开关;打开、固定、更新、删除收进同一个更多菜单。
  • 来源与作用域改为行内辅助信息,skills.css 从 618 行缩减到 258 行,并补齐空列表、已安装、内置、可更新、已停用和窄窗口 Storybook 场景及相关测试。
  • 新增可更新场景的真实交互测试:打开行菜单与差异面板、应用来源版本,并校验传入更新回调的 current/source SHA。
  • 本分支已重放到最新主线,不会重新引入已删除的“创建示例 Skill”入口。

@me2seeks

me2seeks commented Aug 3, 2026

Copy link
Copy Markdown
ContributorAuthor

The Skills-specific checks are passing. Storybook covered the installed view at wide, compact, and floor sizes, along with all five new catalog states. All three Skills E2E tests also passed in shard 2.

The red checks come from the Settings changes in #1972: Storybook fails on daily-review-model-selector-open-narrow, and shard 2 fails only on e2e/settings.spec.ts while focusing the theme radio group. #1972 has the same failures in its own CI run, and the aggregate e2e check simply reflects the shard result.

@Astro-HanAstro-Han 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.

Review: MERGE-READY ✅ (2 independent deepseek-v4-flash passes)

Reviewed the full diff (+564/−703, 9 files), ran the UI suite, and verified the Astryx contracts.

  • Feature parity preserved: install (market/built-in), import sources, enable/disable, direct Use, open SKILL.md, pin, update review, delete (now modal-confirm — safer than the old two-stage timer), refresh, open directory, context check, search/filter/sort, empty state, slug-collision display, exception badge, actionBusy guard — all retained; only the promo banner (removed per #1901) and row hover background were dropped.
  • Astryx className pitfall checked and clear: Item/DropdownMenuItem append consumer className/style via mergePropsafter internal stylex classes (no replace-semantics trap, unlike the pattern that broke #1738's search input).
  • Deletion is clean: the 3 removed CSS tokens and 7 removed copy keys have zero remaining consumers repo-wide (verified by grep); new CSS classes all have consumers; check-story-annotations passes.
  • Tests verified: new skills-panel.test.tsx (4 cases) passes; whole @maka/ui suite 272/272 green; rewritten skill-delete-scope.spec.ts + new skills.spec.ts aria/role assertions match Astryx's real DOM contracts. No "lying" old tests.
  • State discipline improved: timer-based delete-confirm effect (which violated the repo's "Effects only for external sync" convention) removed in favor of event-handler-driven dialog flow.

P2 to handle or explicitly defer: the update-review render journey (menu item → diff panel → apply) was rewritten but has zero committed test coverage (existing tests stop at menuitem existence; no story opens the UPDATE_AVAILABLE_PREVIEW fixture; PR body notes only manual exercise). Recommend adding one render test asserting the review panel opens and "update to source" calls onUpdateManagedSkill with the right sha, or record an explicit deferral.

Non-blocking P3: duplicate rAF-delay helper (skills-panel.tsx:112 vs :781); no in-progress indicator during delete; hover tooltip narrowed to label; hidden .maka-skill-tool-summary-hidden span retained.

Merge prerequisites: (1) the branch is ~100 commits behind main (merge-base 8e5008e vs main 1caea26; GitHub reports MERGEABLE) — CI evidence ran against the old base, so please rebase onto latest main and re-run CI; (2) the existing reds on the old base (settings theme focus flake, daily-review storybook contract) are unrelated to this PR and should disappear on a fresh run.

@me2seeks
me2seeksforce-pushed the fix/1901-skills-astryx-redesign branch from 0bf7b71 to 26dc5a8CompareAugust 4, 2026 13:40
@me2seeks

Copy link
Copy Markdown
ContributorAuthor

Rebased this onto current main (bc12cddf) and added the missing update-review render journey in 26dc5a81. The Storybook play now opens the row menu and diff panel, applies the source update, and asserts the exact current/source SHA arguments. All fresh checks are green (9 passed, 2 skipped by path). @Astro-Han, when you have a moment, could you take another look?

@Astro-Han

Copy link
Copy Markdown
Contributor

LGTM! merging.

@Astro-Han
Astro-Han merged commit 16f1470 into apache:mainAug 4, 2026
11 checks passed
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.

fix(ui): redesign the Skills page on Astryx primitives

2 participants

@me2seeks@Astro-Han
, '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

refactor(ui): simplify the Skills page on Astryx - #1973

Merged
Astro-Han merged 3 commits into
apache:mainfrom
me2seeks:fix/1901-skills-astryx-redesign
Aug 4, 2026
Merged

refactor(ui): simplify the Skills page on Astryx#1973
Astro-Han merged 3 commits into
apache:mainfrom
me2seeks:fix/1901-skills-astryx-redesign

Conversation

@me2seeks

@me2seeksme2seeks commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Summary

  • replace the promotional/card stack and hand-built Skill rows with Astryx Item surfaces
  • keep one direct Use action and one state switch per installed Skill, with open, pin, update, and delete in a single contextual menu
  • keep source and scope as inline supporting metadata, reduce skills.css from 618 to 258 lines, and add the requested Storybook states plus UI/Desktop coverage

Rationale

The page gave promotional content, discovery, context diagnostics, and every row action similar visual weight. This makes discovery and installed management easier to scan while preserving source import, installation, update review, enablement, pinning, deletion, file opening, and explicit invocation.

This is rebased onto current main and does not restore the removed sample-Skill action. The remaining page-level open-folder and refresh actions move into one secondary menu.

Verification

  • npm --workspace @maka/ui test — 321 passed
  • npm --workspace @maka/desktop run typecheck:stories
  • npm --workspace @maka/desktop run build-storybook
  • npm --workspace @maka/desktop run smoke:storybook — 68 manifest checks and 78 catalog renders passed
  • npm --workspace @maka/desktop run test:checks
  • workspace dependency build across core, storage, MCP, runtime, runtime host, computer use, and UI
  • Storybook annotation and visual-smoke harness tests — 9 passed
  • targeted Playwright collection — 3 tests across the Skills hierarchy and scope-aware deletion journeys
  • committed update-review Storybook journey opens the row menu and diff panel, applies the source update, and asserts the exact current/source SHA arguments

The committed Playwright specs use normal pointer interactions. The update-review journey also runs through the real Astryx menu layer in the Storybook browser smoke rather than asserting static menu text only.

Closes#1901

中文说明
  • 使用 Astryx Item 重组市场、内置和已安装 Skill 列表,移除促销区与重复的卡片框架。
  • 每个已安装 Skill 只保留一个直接“使用”动作和一个启停开关;打开、固定、更新、删除收进同一个更多菜单。
  • 来源与作用域改为行内辅助信息,skills.css 从 618 行缩减到 258 行,并补齐空列表、已安装、内置、可更新、已停用和窄窗口 Storybook 场景及相关测试。
  • 新增可更新场景的真实交互测试:打开行菜单与差异面板、应用来源版本,并校验传入更新回调的 current/source SHA。
  • 本分支已重放到最新主线,不会重新引入已删除的“创建示例 Skill”入口。

@me2seeks

me2seeks commented Aug 3, 2026

Copy link
Copy Markdown
ContributorAuthor

The Skills-specific checks are passing. Storybook covered the installed view at wide, compact, and floor sizes, along with all five new catalog states. All three Skills E2E tests also passed in shard 2.

The red checks come from the Settings changes in #1972: Storybook fails on daily-review-model-selector-open-narrow, and shard 2 fails only on e2e/settings.spec.ts while focusing the theme radio group. #1972 has the same failures in its own CI run, and the aggregate e2e check simply reflects the shard result.

@Astro-HanAstro-Han 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.

Review: MERGE-READY ✅ (2 independent deepseek-v4-flash passes)

Reviewed the full diff (+564/−703, 9 files), ran the UI suite, and verified the Astryx contracts.

  • Feature parity preserved: install (market/built-in), import sources, enable/disable, direct Use, open SKILL.md, pin, update review, delete (now modal-confirm — safer than the old two-stage timer), refresh, open directory, context check, search/filter/sort, empty state, slug-collision display, exception badge, actionBusy guard — all retained; only the promo banner (removed per #1901) and row hover background were dropped.
  • Astryx className pitfall checked and clear: Item/DropdownMenuItem append consumer className/style via mergePropsafter internal stylex classes (no replace-semantics trap, unlike the pattern that broke #1738's search input).
  • Deletion is clean: the 3 removed CSS tokens and 7 removed copy keys have zero remaining consumers repo-wide (verified by grep); new CSS classes all have consumers; check-story-annotations passes.
  • Tests verified: new skills-panel.test.tsx (4 cases) passes; whole @maka/ui suite 272/272 green; rewritten skill-delete-scope.spec.ts + new skills.spec.ts aria/role assertions match Astryx's real DOM contracts. No "lying" old tests.
  • State discipline improved: timer-based delete-confirm effect (which violated the repo's "Effects only for external sync" convention) removed in favor of event-handler-driven dialog flow.

P2 to handle or explicitly defer: the update-review render journey (menu item → diff panel → apply) was rewritten but has zero committed test coverage (existing tests stop at menuitem existence; no story opens the UPDATE_AVAILABLE_PREVIEW fixture; PR body notes only manual exercise). Recommend adding one render test asserting the review panel opens and "update to source" calls onUpdateManagedSkill with the right sha, or record an explicit deferral.

Non-blocking P3: duplicate rAF-delay helper (skills-panel.tsx:112 vs :781); no in-progress indicator during delete; hover tooltip narrowed to label; hidden .maka-skill-tool-summary-hidden span retained.

Merge prerequisites: (1) the branch is ~100 commits behind main (merge-base 8e5008e vs main 1caea26; GitHub reports MERGEABLE) — CI evidence ran against the old base, so please rebase onto latest main and re-run CI; (2) the existing reds on the old base (settings theme focus flake, daily-review storybook contract) are unrelated to this PR and should disappear on a fresh run.

@me2seeks
me2seeksforce-pushed the fix/1901-skills-astryx-redesign branch from 0bf7b71 to 26dc5a8CompareAugust 4, 2026 13:40
@me2seeks

Copy link
Copy Markdown
ContributorAuthor

Rebased this onto current main (bc12cddf) and added the missing update-review render journey in 26dc5a81. The Storybook play now opens the row menu and diff panel, applies the source update, and asserts the exact current/source SHA arguments. All fresh checks are green (9 passed, 2 skipped by path). @Astro-Han, when you have a moment, could you take another look?

@Astro-Han

Copy link
Copy Markdown
Contributor

LGTM! merging.

@Astro-Han
Astro-Han merged commit 16f1470 into apache:mainAug 4, 2026
11 checks passed
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.

fix(ui): redesign the Skills page on Astryx primitives

2 participants

@me2seeks@Astro-Han
, '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

refactor(ui): simplify the Skills page on Astryx - #1973

Merged
Astro-Han merged 3 commits into
apache:mainfrom
me2seeks:fix/1901-skills-astryx-redesign
Aug 4, 2026
Merged

refactor(ui): simplify the Skills page on Astryx#1973
Astro-Han merged 3 commits into
apache:mainfrom
me2seeks:fix/1901-skills-astryx-redesign

Conversation

@me2seeks

@me2seeksme2seeks commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Summary

  • replace the promotional/card stack and hand-built Skill rows with Astryx Item surfaces
  • keep one direct Use action and one state switch per installed Skill, with open, pin, update, and delete in a single contextual menu
  • keep source and scope as inline supporting metadata, reduce skills.css from 618 to 258 lines, and add the requested Storybook states plus UI/Desktop coverage

Rationale

The page gave promotional content, discovery, context diagnostics, and every row action similar visual weight. This makes discovery and installed management easier to scan while preserving source import, installation, update review, enablement, pinning, deletion, file opening, and explicit invocation.

This is rebased onto current main and does not restore the removed sample-Skill action. The remaining page-level open-folder and refresh actions move into one secondary menu.

Verification

  • npm --workspace @maka/ui test — 321 passed
  • npm --workspace @maka/desktop run typecheck:stories
  • npm --workspace @maka/desktop run build-storybook
  • npm --workspace @maka/desktop run smoke:storybook — 68 manifest checks and 78 catalog renders passed
  • npm --workspace @maka/desktop run test:checks
  • workspace dependency build across core, storage, MCP, runtime, runtime host, computer use, and UI
  • Storybook annotation and visual-smoke harness tests — 9 passed
  • targeted Playwright collection — 3 tests across the Skills hierarchy and scope-aware deletion journeys
  • committed update-review Storybook journey opens the row menu and diff panel, applies the source update, and asserts the exact current/source SHA arguments

The committed Playwright specs use normal pointer interactions. The update-review journey also runs through the real Astryx menu layer in the Storybook browser smoke rather than asserting static menu text only.

Closes#1901

中文说明
  • 使用 Astryx Item 重组市场、内置和已安装 Skill 列表,移除促销区与重复的卡片框架。
  • 每个已安装 Skill 只保留一个直接“使用”动作和一个启停开关;打开、固定、更新、删除收进同一个更多菜单。
  • 来源与作用域改为行内辅助信息,skills.css 从 618 行缩减到 258 行,并补齐空列表、已安装、内置、可更新、已停用和窄窗口 Storybook 场景及相关测试。
  • 新增可更新场景的真实交互测试:打开行菜单与差异面板、应用来源版本,并校验传入更新回调的 current/source SHA。
  • 本分支已重放到最新主线,不会重新引入已删除的“创建示例 Skill”入口。

@me2seeks

me2seeks commented Aug 3, 2026

Copy link
Copy Markdown
ContributorAuthor

The Skills-specific checks are passing. Storybook covered the installed view at wide, compact, and floor sizes, along with all five new catalog states. All three Skills E2E tests also passed in shard 2.

The red checks come from the Settings changes in #1972: Storybook fails on daily-review-model-selector-open-narrow, and shard 2 fails only on e2e/settings.spec.ts while focusing the theme radio group. #1972 has the same failures in its own CI run, and the aggregate e2e check simply reflects the shard result.

@Astro-HanAstro-Han 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.

Review: MERGE-READY ✅ (2 independent deepseek-v4-flash passes)

Reviewed the full diff (+564/−703, 9 files), ran the UI suite, and verified the Astryx contracts.

  • Feature parity preserved: install (market/built-in), import sources, enable/disable, direct Use, open SKILL.md, pin, update review, delete (now modal-confirm — safer than the old two-stage timer), refresh, open directory, context check, search/filter/sort, empty state, slug-collision display, exception badge, actionBusy guard — all retained; only the promo banner (removed per #1901) and row hover background were dropped.
  • Astryx className pitfall checked and clear: Item/DropdownMenuItem append consumer className/style via mergePropsafter internal stylex classes (no replace-semantics trap, unlike the pattern that broke #1738's search input).
  • Deletion is clean: the 3 removed CSS tokens and 7 removed copy keys have zero remaining consumers repo-wide (verified by grep); new CSS classes all have consumers; check-story-annotations passes.
  • Tests verified: new skills-panel.test.tsx (4 cases) passes; whole @maka/ui suite 272/272 green; rewritten skill-delete-scope.spec.ts + new skills.spec.ts aria/role assertions match Astryx's real DOM contracts. No "lying" old tests.
  • State discipline improved: timer-based delete-confirm effect (which violated the repo's "Effects only for external sync" convention) removed in favor of event-handler-driven dialog flow.

P2 to handle or explicitly defer: the update-review render journey (menu item → diff panel → apply) was rewritten but has zero committed test coverage (existing tests stop at menuitem existence; no story opens the UPDATE_AVAILABLE_PREVIEW fixture; PR body notes only manual exercise). Recommend adding one render test asserting the review panel opens and "update to source" calls onUpdateManagedSkill with the right sha, or record an explicit deferral.

Non-blocking P3: duplicate rAF-delay helper (skills-panel.tsx:112 vs :781); no in-progress indicator during delete; hover tooltip narrowed to label; hidden .maka-skill-tool-summary-hidden span retained.

Merge prerequisites: (1) the branch is ~100 commits behind main (merge-base 8e5008e vs main 1caea26; GitHub reports MERGEABLE) — CI evidence ran against the old base, so please rebase onto latest main and re-run CI; (2) the existing reds on the old base (settings theme focus flake, daily-review storybook contract) are unrelated to this PR and should disappear on a fresh run.

@me2seeks
me2seeksforce-pushed the fix/1901-skills-astryx-redesign branch from 0bf7b71 to 26dc5a8CompareAugust 4, 2026 13:40
@me2seeks

Copy link
Copy Markdown
ContributorAuthor

Rebased this onto current main (bc12cddf) and added the missing update-review render journey in 26dc5a81. The Storybook play now opens the row menu and diff panel, applies the source update, and asserts the exact current/source SHA arguments. All fresh checks are green (9 passed, 2 skipped by path). @Astro-Han, when you have a moment, could you take another look?

@Astro-Han

Copy link
Copy Markdown
Contributor

LGTM! merging.

@Astro-Han
Astro-Han merged commit 16f1470 into apache:mainAug 4, 2026
11 checks passed
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.

fix(ui): redesign the Skills page on Astryx primitives

2 participants

@me2seeks@Astro-Han
, '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

refactor(ui): simplify the Skills page on Astryx - #1973

Merged
Astro-Han merged 3 commits into
apache:mainfrom
me2seeks:fix/1901-skills-astryx-redesign
Aug 4, 2026
Merged

refactor(ui): simplify the Skills page on Astryx#1973
Astro-Han merged 3 commits into
apache:mainfrom
me2seeks:fix/1901-skills-astryx-redesign

Conversation

@me2seeks

@me2seeksme2seeks commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Summary

  • replace the promotional/card stack and hand-built Skill rows with Astryx Item surfaces
  • keep one direct Use action and one state switch per installed Skill, with open, pin, update, and delete in a single contextual menu
  • keep source and scope as inline supporting metadata, reduce skills.css from 618 to 258 lines, and add the requested Storybook states plus UI/Desktop coverage

Rationale

The page gave promotional content, discovery, context diagnostics, and every row action similar visual weight. This makes discovery and installed management easier to scan while preserving source import, installation, update review, enablement, pinning, deletion, file opening, and explicit invocation.

This is rebased onto current main and does not restore the removed sample-Skill action. The remaining page-level open-folder and refresh actions move into one secondary menu.

Verification

  • npm --workspace @maka/ui test — 321 passed
  • npm --workspace @maka/desktop run typecheck:stories
  • npm --workspace @maka/desktop run build-storybook
  • npm --workspace @maka/desktop run smoke:storybook — 68 manifest checks and 78 catalog renders passed
  • npm --workspace @maka/desktop run test:checks
  • workspace dependency build across core, storage, MCP, runtime, runtime host, computer use, and UI
  • Storybook annotation and visual-smoke harness tests — 9 passed
  • targeted Playwright collection — 3 tests across the Skills hierarchy and scope-aware deletion journeys
  • committed update-review Storybook journey opens the row menu and diff panel, applies the source update, and asserts the exact current/source SHA arguments

The committed Playwright specs use normal pointer interactions. The update-review journey also runs through the real Astryx menu layer in the Storybook browser smoke rather than asserting static menu text only.

Closes#1901

中文说明
  • 使用 Astryx Item 重组市场、内置和已安装 Skill 列表,移除促销区与重复的卡片框架。
  • 每个已安装 Skill 只保留一个直接“使用”动作和一个启停开关;打开、固定、更新、删除收进同一个更多菜单。
  • 来源与作用域改为行内辅助信息,skills.css 从 618 行缩减到 258 行,并补齐空列表、已安装、内置、可更新、已停用和窄窗口 Storybook 场景及相关测试。
  • 新增可更新场景的真实交互测试:打开行菜单与差异面板、应用来源版本,并校验传入更新回调的 current/source SHA。
  • 本分支已重放到最新主线,不会重新引入已删除的“创建示例 Skill”入口。

@me2seeks

me2seeks commented Aug 3, 2026

Copy link
Copy Markdown
ContributorAuthor

The Skills-specific checks are passing. Storybook covered the installed view at wide, compact, and floor sizes, along with all five new catalog states. All three Skills E2E tests also passed in shard 2.

The red checks come from the Settings changes in #1972: Storybook fails on daily-review-model-selector-open-narrow, and shard 2 fails only on e2e/settings.spec.ts while focusing the theme radio group. #1972 has the same failures in its own CI run, and the aggregate e2e check simply reflects the shard result.

@Astro-HanAstro-Han 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.

Review: MERGE-READY ✅ (2 independent deepseek-v4-flash passes)

Reviewed the full diff (+564/−703, 9 files), ran the UI suite, and verified the Astryx contracts.

  • Feature parity preserved: install (market/built-in), import sources, enable/disable, direct Use, open SKILL.md, pin, update review, delete (now modal-confirm — safer than the old two-stage timer), refresh, open directory, context check, search/filter/sort, empty state, slug-collision display, exception badge, actionBusy guard — all retained; only the promo banner (removed per #1901) and row hover background were dropped.
  • Astryx className pitfall checked and clear: Item/DropdownMenuItem append consumer className/style via mergePropsafter internal stylex classes (no replace-semantics trap, unlike the pattern that broke #1738's search input).
  • Deletion is clean: the 3 removed CSS tokens and 7 removed copy keys have zero remaining consumers repo-wide (verified by grep); new CSS classes all have consumers; check-story-annotations passes.
  • Tests verified: new skills-panel.test.tsx (4 cases) passes; whole @maka/ui suite 272/272 green; rewritten skill-delete-scope.spec.ts + new skills.spec.ts aria/role assertions match Astryx's real DOM contracts. No "lying" old tests.
  • State discipline improved: timer-based delete-confirm effect (which violated the repo's "Effects only for external sync" convention) removed in favor of event-handler-driven dialog flow.

P2 to handle or explicitly defer: the update-review render journey (menu item → diff panel → apply) was rewritten but has zero committed test coverage (existing tests stop at menuitem existence; no story opens the UPDATE_AVAILABLE_PREVIEW fixture; PR body notes only manual exercise). Recommend adding one render test asserting the review panel opens and "update to source" calls onUpdateManagedSkill with the right sha, or record an explicit deferral.

Non-blocking P3: duplicate rAF-delay helper (skills-panel.tsx:112 vs :781); no in-progress indicator during delete; hover tooltip narrowed to label; hidden .maka-skill-tool-summary-hidden span retained.

Merge prerequisites: (1) the branch is ~100 commits behind main (merge-base 8e5008e vs main 1caea26; GitHub reports MERGEABLE) — CI evidence ran against the old base, so please rebase onto latest main and re-run CI; (2) the existing reds on the old base (settings theme focus flake, daily-review storybook contract) are unrelated to this PR and should disappear on a fresh run.

@me2seeks
me2seeksforce-pushed the fix/1901-skills-astryx-redesign branch from 0bf7b71 to 26dc5a8CompareAugust 4, 2026 13:40
@me2seeks

Copy link
Copy Markdown
ContributorAuthor

Rebased this onto current main (bc12cddf) and added the missing update-review render journey in 26dc5a81. The Storybook play now opens the row menu and diff panel, applies the source update, and asserts the exact current/source SHA arguments. All fresh checks are green (9 passed, 2 skipped by path). @Astro-Han, when you have a moment, could you take another look?

@Astro-Han

Copy link
Copy Markdown
Contributor

LGTM! merging.

@Astro-Han
Astro-Han merged commit 16f1470 into apache:mainAug 4, 2026
11 checks passed
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.

fix(ui): redesign the Skills page on Astryx primitives

2 participants

@me2seeks@Astro-Han