fix(soup): hide disabled context menu items#2185
Merged
evanhutnik merged 5 commits intomainfrom Mar 25, 2026
Merged
Conversation
Wrap each MenuItem in a <Show> conditional so that actions unavailable for the selected entity are omitted from the right-click menu entirely, rather than appearing as disabled/greyed-out entries. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughSoupEntityActionsMenu now omits ineligible menu items and conditional dividers entirely, replacing previously rendered-but-disabled items with SolidJS Changes
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@js/app/packages/app/component/next-soup/soup-view/soup-entity-actions-menu.tsx`:
- Around line 125-186: The two Divider components are always rendered causing
leading/trailing or consecutive dividers when some action groups are hidden;
compute visibility booleans for each group (e.g., canShowRename =
canExecuteAll(renameAction.canExecute), canShowMove =
canExecuteAny(moveToProjectAction.canExecute), canShowCopy =
canExecuteAny(copyAction.canExecute), canShowCopyLink = props.entities.length
=== 1, canShowCopyBranch = props.entities.length === 1 &&
copyBranchNameAction.canExecute(props.entities[0]), canShowShare =
props.entities.length === 1 && shareAction.canExecute(props.entities[0]),
canShowBlock = canExecuteAll(blockSenderAction.canExecute)) and only render a
Divider when the adjacent groups on both sides are visible (wrap the first
Divider in a conditional that checks if any of the items after it are visible,
and the second Divider only if any items before it are visible) so no orphan or
back-to-back separators are produced; use these booleans in place of the current
unconditional Divider renders and keep existing MenuItem onClick handlers like
handleAction(renameAction.executeWithSoup).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 03be9a56-0894-4ac7-8e84-37b05bb21acd
📒 Files selected for processing (1)
js/app/packages/app/component/next-soup/soup-view/soup-entity-actions-menu.tsx
js/app/packages/app/component/next-soup/soup-view/soup-entity-actions-menu.tsx
Outdated
Show resolved
Hide resolved
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
js/app/packages/app/component/next-soup/soup-view/soup-entity-actions-menu.tsx (1)
122-183:⚠️ Potential issue | 🟡 MinorGuard separators so hidden groups don't leave orphan or back-to-back dividers.
The
<Divider />at lines 122 and 183 are unconditionally rendered. When all items in a group are hidden (e.g., no email entities selected → no "Block Sender", single non-shareable doc → no "Share"), this produces orphan leading/trailing or consecutive dividers.Compute visibility predicates for each group and wrap dividers accordingly:
💡 Proposed fix
+ const hasTopGroupItems = () => + canExecuteAny(markDone.canExecute) || canOpenInSplit(); + + const hasMiddleGroupItems = () => + canExecuteAll(renameAction.canExecute) || + canExecuteAny(moveToProjectAction.canExecute) || + canExecuteAny(copyAction.canExecute) || + props.entities.length === 1 || + canExecuteAll(blockSenderAction.canExecute); + + const hasDeleteItem = () => canExecuteAll(deleteAction.canExecute); return ( <> <Show when={canExecuteAny(markDone.canExecute)}> ... </Show> <Show when={canOpenInSplit()}> ... </Show> - <Divider /> + <Show when={hasTopGroupItems() && hasMiddleGroupItems()}> + <Divider /> + </Show> ... middle group items ... - <Divider /> + <Show when={hasMiddleGroupItems() && hasDeleteItem()}> + <Divider /> + </Show> <Show when={canExecuteAll(deleteAction.canExecute)}> ... </Show> </> );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/app/packages/app/component/next-soup/soup-view/soup-entity-actions-menu.tsx` around lines 122 - 183, The two Divider components are always rendered and can create orphan or back-to-back dividers when their surrounding MenuItem groups are all hidden; wrap each Divider in a conditional that checks whether at least one item in the adjacent group will be shown (e.g., compute predicates using canExecuteAll/canExecuteAny and per-entity checks such as renameAction.canExecute, moveToProjectAction.canExecute, copyAction.canExecute, copyLinkAction (props.entities.length === 1), copyBranchNameAction.canExecute(props.entities[0]), shareAction.canExecute(props.entities[0]), blockSenderAction.canExecute) and only render the Divider when the corresponding groupVisible predicate is true; update the rendering around the top Divider and the bottom Divider to use those group visibility predicates instead of unconditional Divider rendering.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@js/app/packages/app/component/next-soup/soup-view/soup-entity-actions-menu.tsx`:
- Around line 122-183: The two Divider components are always rendered and can
create orphan or back-to-back dividers when their surrounding MenuItem groups
are all hidden; wrap each Divider in a conditional that checks whether at least
one item in the adjacent group will be shown (e.g., compute predicates using
canExecuteAll/canExecuteAny and per-entity checks such as
renameAction.canExecute, moveToProjectAction.canExecute, copyAction.canExecute,
copyLinkAction (props.entities.length === 1),
copyBranchNameAction.canExecute(props.entities[0]),
shareAction.canExecute(props.entities[0]), blockSenderAction.canExecute) and
only render the Divider when the corresponding groupVisible predicate is true;
update the rendering around the top Divider and the bottom Divider to use those
group visibility predicates instead of unconditional Divider rendering.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 42b8eae6-2bd9-48b9-90a9-4eaca2228162
📒 Files selected for processing (1)
js/app/packages/app/component/next-soup/soup-view/soup-entity-actions-menu.tsx
Contributor
weird divider at bottom |
Dividers between action groups are now only rendered when both adjacent groups have at least one visible item, preventing orphan or consecutive dividers when action groups are fully hidden. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@js/app/packages/app/component/next-soup/soup-view/soup-entity-actions-menu.tsx`:
- Around line 134-136: The Divider rendering logic can produce two adjacent
dividers; update the second <Show> condition so the Divider only appears when
the middle group is visible (and delete group follows). Locate the second
Divider wrapped in a <Show> that currently uses showTopGroup() ||
showMiddleGroup() (or similar) and change it to require showMiddleGroup() &&
showDeleteGroup(), keeping the first Divider (the one using showTopGroup() &&
(showMiddleGroup() || showDeleteGroup())) unchanged; references: showTopGroup(),
showMiddleGroup(), showDeleteGroup(), and the <Divider /> inside the <Show>
wrapper.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 52417486-5bc1-4ecc-88cc-efa17365b8ff
📒 Files selected for processing (1)
js/app/packages/app/component/next-soup/soup-view/soup-entity-actions-menu.tsx
js/app/packages/app/component/next-soup/soup-view/soup-entity-actions-menu.tsx
Show resolved
Hide resolved
The second divider now only renders when the middle group is visible, preventing back-to-back dividers when only top and delete groups show. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This was referenced Apr 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
MenuItemis wrapped in a SolidJS<Show>conditional so only actionable options appearTest plan
🤖 Generated with Claude Code