Compact automation filter menus - #997
Conversation
|
🚨 SLOP COP 🚨 · I am SlopCop. I am reviewing this pull request under the The stack contains #993, #994, #995, #996, #997, #1000, and #1001. All seven pull requests are ready for review, and @brsbl has write access. I will run security, code quality, performance, architecture, and end-to-end checks. I will post each phase result when it completes. |
|
🚨 SLOP COP 🚨 · I am the SlopCop. I am reviewing this pull request under the The stack contains #994, #995, #996, #997, #1000, and #1001. I will review each change separately. I will judge each change against the final stack state. I found no skipped pull requests. All stack pull requests are ready. The author has write access. I started parallel security, code quality, and performance reviews. I will test the top pull request once through the application, if possible. |
There was a problem hiding this comment.
🚨 SLOP COP 🚨 · review
I am SlopCop. I am reviewing this pull request with the security, code quality, performance, architecture, and end-to-end guidelines.
The stack contains #995, #996, #997, #1000, and #1001, from bottom to top. I will judge each pull request after the full stack lands. I will run the end-to-end test once on #1001.
|
🚨 SLOP COP 🚨 · I am SlopCop. I am reviewing this pull request under the configured security, quality, performance, architecture, and end-to-end test rules. The stack contains these pull requests, from bottom to top:
I will judge each pull request against the final state after the complete stack lands. I will run the end-to-end test on #1001 only. |
|
🚨 SLOP COP 🚨 · I am SlopCop. I am reviewing this stack under the The stack contains these pull requests, from bottom to top:
I will run security, code quality and architecture, performance, and one end-to-end phase on top PR #1001. I skipped no pull requests. |
SawyerHood
left a comment
There was a problem hiding this comment.
🚨 SLOP COP 🚨 · review
Security phase complete for #997.
The security phase is complete. I found no security issues in this pull request after the full stack lands.
The changes add no untrusted input paths, HTML injection, network calls, file access, process execution, or credential handling.
SawyerHood
left a comment
There was a problem hiding this comment.
🚨 SLOP COP 🚨 · review
Performance phase complete for #997.
The performance phase is complete. I found no performance or resource-use issues in this pull request after the full stack lands.
The changes add no loops, subscriptions, timers, network calls, or persistent data structures. The React and layout work stays small.
|
🚨 SLOP COP 🚨 · Security phase complete. I found no security issue in this PR after I reviewed the complete stack. The changes do not add a trust-boundary change, unsafe render sink, data exposure, dependency risk, or protocol change. |
|
🚨 SLOP COP 🚨 · Security phase complete: no finding. The change uses typed icon names and literal class names. React escapes project labels. The menu width has a fixed maximum. The change adds no route, authorization, data, dependency, or host protocol surface. |
|
🚨 SLOP COP 🚨 · Performance phase complete. I found no performance or resource-use defects in this pull request. I reviewed it against the final seven-pull-request stack. I checked render work, value stability, filtering, sorting, memoization, requests, subscriptions, and test cost. |
|
🚨 SLOP COP 🚨 · Security phase complete. I found no security or privacy defects in this pull request. I reviewed it against the final seven-pull-request stack. I checked unsafe rendering, injection, links, actions, permissions, data disclosure, configuration, dependencies, and trust boundaries. |
|
🚨 SLOP COP 🚨 · Security phase complete. No security or privacy findings. The compact flags and typed icon names only select fixed presentation values. Validation included the complete stacked diff, trust-boundary tracing, and unsafe-render checks. |
| <ResourceMenuTrigger label="Sort" icon="ArrowUpDown" /> | ||
| <DropdownMenuContent align="end" mobileTitle="Sort" className="min-w-40"> | ||
| <DropdownMenuLabel className="text-xs font-normal text-subtle-foreground"> | ||
| <ResourceMenuTrigger |
There was a problem hiding this comment.
🚨 slopcop/review — Expose the active sort direction to assistive technology.
The icon now shows the direction, but the trigger name and tooltip stay as “Sort.” A screen reader cannot identify ascending or descending state.
Please include the direction in the accessible label and tooltip.
| <DropdownMenuContent | ||
| align="end" | ||
| mobileTitle={label} | ||
| className={cn( |
There was a problem hiding this comment.
🚨 slopcop/review — Format the changed files before merge.
The Prettier check fails for this file and plugins/automations/overview-view.tsx. The base versions pass the same check.
Please run the repository formatter for both files.
|
🚨 SLOP COP 🚨 · Code quality and architecture phase complete. I found two issues. The sort direction has no accessible state. Prettier also rejects two changed files. The compact API otherwise reuses the shared resource toolbar. The filter and sort state logic remains correct. |
|
🚨 SLOP COP 🚨 · Performance phase complete: no finding. The menu content mounts only when open. Static options initialize once. Project options remain memoized. Other call sites keep the default density. |
|
🚨 SLOP COP 🚨 · Performance phase complete. No performance findings. Constant and memoized icons avoid repeated work, and intrinsic menu sizing only runs when the menu opens. Validation covered render cost, repeated work, layout, test runtime, and bundle impact. |
| <DropdownMenuLabel className="text-xs font-normal text-subtle-foreground"> | ||
| <ResourceMenuTrigger | ||
| label="Sort" | ||
| icon={direction === "asc" ? "ArrowUp" : "ArrowDown"} |
There was a problem hiding this comment.
🚨 slopcop/review — The sort direction has no accessible text.
The icon shows the direction, but the component hides the icon from assistive technology. The accessible label and tooltip still say only Sort.
Add the direction to the accessible label and tooltip. For example, use Sort: ascending and Sort: descending.
SawyerHood
left a comment
There was a problem hiding this comment.
🚨 SLOP COP 🚨 · review
Result: I found one medium accessibility issue.
The sort icon now shows the direction. The icon remains hidden from assistive technology. The accessible label and tooltip do not contain the direction.
This shared change also affects all ResourceSortMenu users. The pull request tests only the Automations user. I found no material performance regression. Current CI passed.
Inline finding: #997 (comment)
| <DropdownMenuContent align="end" mobileTitle="Sort" className="min-w-40"> | ||
| <DropdownMenuLabel className="text-xs font-normal text-subtle-foreground"> | ||
| <ResourceMenuTrigger | ||
| label="Sort" |
There was a problem hiding this comment.
🚨 slopcop/review — The sort field and direction have no accessible state.
The trigger label stays Sort. Both direction icons use aria-hidden, and the menu items expose no selected state.
Include the field and direction in the trigger name. Use radio menu items with a checked state.
SawyerHood
left a comment
There was a problem hiding this comment.
🚨 SLOP COP 🚨 · review
Review complete.
I found one low accessibility issue. The sort trigger does not expose the selected field or direction.
The security and performance phases found no issues. The final GPT-5.6 gate confirmed this finding.
I used a comment-only review. I did not approve or request changes.
|
🚨 SLOP COP 🚨 · Code-quality and architecture phase complete. No blocking code-quality finding. The shared Sort trigger change matches the stated direction indicator. The compact class duplication is small and local. I checked correctness, tests, duplication, architectural fit, stale names, and later stack fixes. |
SawyerHood
left a comment
There was a problem hiding this comment.
🚨 SLOP COP 🚨 · review
Code quality and architecture phase complete for #997.
I found no actionable code quality, correctness, test, duplication, or architecture issue after the full stack lands. The pull request explicitly changes the shared Sort trigger to show direction. The compact prop provides one clear opt-in density without duplicate menu code.
SawyerHood
left a comment
There was a problem hiding this comment.
🚨 SLOP COP 🚨 · review
Final review for #997: no findings.
The security, code quality, architecture, duplication, and performance reviews found no actionable issue after the full stack lands. The one top-stack browser test passed through the Installed Plugins route and the Automations detail page.
All GitHub CI jobs passed on the top pull request. I did not approve or request changes, and I posted no inline findings.
| <ResourceMenuTrigger label="Sort" icon="ArrowUpDown" /> | ||
| <DropdownMenuContent align="end" mobileTitle="Sort" className="min-w-40"> | ||
| <DropdownMenuLabel className="text-xs font-normal text-subtle-foreground"> | ||
| <ResourceMenuTrigger |
There was a problem hiding this comment.
🚨 slopcop/review — The sort direction has no accessible text.
The trigger changes an aria-hidden icon between up and down, while its accessible name remains Sort. A screen reader cannot identify the current direction. Please include ascending or descending in the accessible name and tooltip.
SawyerHood
left a comment
There was a problem hiding this comment.
🚨 SLOP COP 🚨 · review
Final comment-only review for #997.
One medium finding.
The sort trigger shows direction only through an icon that assistive tools cannot read. I posted the finding on the changed line.
The security review found no defect. The performance review found no defect. The architecture scan found no additional duplication.
I ran the end-to-end attempt only on the top pull request. The shared host did not produce a ready development server.
GitHub CI passed on the latest stack head. The GPT-5.6 review gate approved this review result.
|
🚨 SLOP COP 🚨 · Performance phase complete. I found no material performance issue in this PR after I reviewed the complete stack. The render scope, allocations, bundle inputs, and pagination behavior stay stable. |
| <DropdownMenuLabel className="text-xs font-normal text-subtle-foreground"> | ||
| <ResourceMenuTrigger | ||
| label="Sort" | ||
| icon={direction === "asc" ? "ArrowUp" : "ArrowDown"} |
There was a problem hiding this comment.
🚨 slopcop/review — The direction icon change affects three unrelated sort menus.
Only Automations enables compact, but this icon change has no compact condition. The browser test shows ArrowUp in Skills too.
Keep this change inside the compact mode, or update and test all four surfaces. Include the direction in the accessible label.
| align="end" | ||
| mobileTitle={label} | ||
| className={cn( | ||
| compact ? "w-max max-w-64 md:p-0.5" : "min-w-44", |
There was a problem hiding this comment.
🚨 slopcop/review — The width cap hides long project names without a readable alternative.
The option label already uses truncate. This cap can hide ordinary project names, and the item has no tooltip.
Keep the full name readable, or add a tooltip before you add the cap.
|
🚨 SLOP COP 🚨 · Code-quality and architecture phase complete. I left two behavior findings. The width cap hides long names, and the sort icon change affects unrelated surfaces. I checked the full stack and checked later PRs before I reported each issue. |
SawyerHood
left a comment
There was a problem hiding this comment.
🚨 SLOP COP 🚨 · review
Review complete with two findings.
- Medium: the Sort trigger shows direction visually but does not expose it in the accessible name.
- Low: Prettier rejects two changed files.
Security and performance checks found no issue. The browser test verified the compact menu widths, icons, disabled state, and direction changes. All GitHub checks pass.
SawyerHood
left a comment
There was a problem hiding this comment.
🚨 SLOP COP 🚨 · review
No findings.
Security and performance reviews found no problems. The shared Sort trigger change matches the stated direction indicator.
The focused automation suite passed 8 tests. All GitHub checks passed. Browser QA confirmed compact menu classes, option icons, disabled state, and direction icons.
SawyerHood
left a comment
There was a problem hiding this comment.
🚨 SLOP COP 🚨 · review
Review result: two behavior findings.
- Security review found no issue.
- Performance review found no material render, allocation, bundle, or pagination issue.
- Turbo type checks passed for the app, shared UI, Automations, and Docs packages.
- The affected tests passed after clean reruns. The latest Docs suite passed all 57 tests.
The menu can hide long project names. The direction icon also changed three unrelated sort menus.
## Summary - keep the Docs side-panel collapse control in the shared page/pane header, separate from the plugin-owned panel toolbar - reuse one toggle component for host-header and embedded fallback layouts - keep the header segment aligned to the resizable right panel and paint both with the themed sidebar surface - cover expanded, collapsed, restored, and fallback placement behavior ## Verification - `pnpm exec turbo run test --filter=bb-plugin-simple-notes -- --run app.test.tsx` (22 passed) - `pnpm exec turbo run typecheck --filter=bb-plugin-simple-notes` - branch desktop app: full-page Docs expanded/collapsed/restored; header separation and surface continuity verified
19e12f0
into
bb/plugin-release-label-emphasis-thr_cxz5rwzns3
Summary
Verification
pnpm exec turbo run test --filter=@bb/app -- --run src/components/tools/automation-overview.test.tsx(8 tests passed)pnpm exec turbo run typecheck --filter=@bb/shared-ui --filter=@bb/appStack