Hide redundant notebook download submenu - #10607
Merged
Merged
Conversation
The export dialog contains every notebook download format, so the separate submenu repeats the same choices. The command palette keeps the direct format actions.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
All contributors have signed the CLA ✍️ ✅ |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR removes the redundant “Download” submenu from the notebook menu (since export covers the same formats), while keeping the per-format download actions available through the command palette.
Changes:
- Mark the notebook “Download” action as redundant so it’s omitted from the notebook menu dropdown.
- Update notebook menu dropdown tests to validate the menu no longer shows “Download” and to trigger download actions via the command palette.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| frontend/src/components/editor/controls/tests/notebook-menu-dropdown.test.tsx | Adjusts tests to assert “Download” is hidden in the menu and uses CommandPalette-based actions; adds a jsdom workaround for scrollIntoView. |
| frontend/src/components/editor/actions/useNotebookActions.tsx | Marks the “Download” parent action as redundant to hide it from the notebook dropdown UI surface. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+36
to
+38
| global.HTMLElement.prototype.scrollIntoView = () => { | ||
| // jsdom does not implement scrollIntoView; cmdk calls it on selection. | ||
| }; |
Comment on lines
143
to
151
| it("opens the slides PDF shortcut with the slides layout", async () => { | ||
| store.set(layoutStateAtom, { | ||
| selectedLayout: "slides", | ||
| layoutData: {}, | ||
| }); | ||
| render(<NotebookMenuDropdown />, { wrapper }); | ||
| renderNotebookControls(); | ||
|
|
||
| await openDownloadMenu(); | ||
| fireEvent.click( | ||
| await screen.findByRole("menuitem", { | ||
| name: "Download as PDF", | ||
| }), | ||
| ); | ||
| fireEvent.click( | ||
| await screen.findByRole("menuitem", { | ||
| name: /Slides Layout/, | ||
| }), | ||
| ); | ||
| await selectDownloadCommand("Download as PDF > Slides Layout"); | ||
|
|
Comment on lines
190
to
196
| { | ||
| icon: <DownloadIcon size={14} strokeWidth={1.5} />, | ||
| label: "Download", | ||
| redundant: true, | ||
| handle: NOOP_HANDLER, | ||
| dropdown: [ | ||
| { |
Contributor
Coverage Report for ./frontend
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="frontend/src/components/editor/actions/useNotebookActions.tsx">
<violation number="1" location="frontend/src/components/editor/actions/useNotebookActions.tsx:193">
P2: Filter hidden download children before exposing them through the command palette. Otherwise `Slides Layout` remains visible and runnable when `isWasm()` is true, even though the dropdown correctly hides the redundant parent.</violation>
</file>
Architecture diagram
sequenceDiagram
participant UI as Editor UI
participant Menu as NotebookMenuDropdown
participant Actions as useNotebookActions
participant Palette as CommandPalette
participant Dialog as ExportDialog
participant State as Jotai Store
Note over UI,State: Notebook export/download flow
UI->>Menu: Click notebook menu button
Menu->>Actions: Get actions list
Actions-->>Menu: Return actions (Download marked redundant)
alt Download action has redundant flag
Menu->>Menu: Filter out redundant submenu
Menu-->>UI: Render menu without Download submenu
else Non-redundant actions
Menu-->>UI: Render menu with all actions
end
Note over UI,Palette: Command palette flow
UI->>Palette: Open command palette
Palette->>State: Read commandPaletteAtom
State-->>Palette: Palette visible
Palette->>Actions: Get available download commands
Actions-->>Palette: Return Download > format commands
Palette-->>UI: Render command list
UI->>Palette: Select Download > Download as HTML (exclude code)
Palette->>State: Set export options (format, code inclusion)
Palette->>State: Set lastExportFormat
State-->>Dialog: Export options configured
Palette-->>Dialog: Open export dialog
Dialog-->>UI: Render export dialog with preselected format
Note over UI,State: PDF slides layout flow
UI->>Palette: Select Download > Download as PDF > Slides Layout
Palette->>State: Set export options (PDF, slides layout)
Palette->>State: Set lastExportFormat
State-->>Dialog: Export options configured
Palette-->>Dialog: Open export dialog
Dialog-->>UI: Render export dialog with slides layout selected
Note over UI,State: WebAssembly environment handling
alt isWasm true
Palette->>Actions: Request download commands
Actions-->>Palette: Filter out slides layout option
Palette-->>UI: Render commands excluding slides
end
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| { | ||
| icon: <DownloadIcon size={14} strokeWidth={1.5} />, | ||
| label: "Download", | ||
| redundant: true, |
Contributor
There was a problem hiding this comment.
P2: Filter hidden download children before exposing them through the command palette. Otherwise Slides Layout remains visible and runnable when isWasm() is true, even though the dropdown correctly hides the redundant parent.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/components/editor/actions/useNotebookActions.tsx, line 193:
<comment>Filter hidden download children before exposing them through the command palette. Otherwise `Slides Layout` remains visible and runnable when `isWasm()` is true, even though the dropdown correctly hides the redundant parent.</comment>
<file context>
@@ -190,6 +190,7 @@ export function useNotebookActions({
{
icon: <DownloadIcon size={14} strokeWidth={1.5} />,
label: "Download",
+ redundant: true,
handle: NOOP_HANDLER,
dropdown: [
</file context>
Contributor
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="frontend/src/components/editor/actions/types.ts">
<violation number="1" location="frontend/src/components/editor/actions/types.ts:48">
P3: The new `hidden` filtering in flattenActions changes which actions appear in the command palette for all cell, notebook and config actions, with non-obvious subtleties: a hidden parent drops its whole subtree (e.g. "Share" when no sharing option is enabled) while `redundant` parents still recurse. None of this is covered by a unit test — flattenActions has no test file — so a regression here (e.g. a hidden parent silently removing runnable children, or a non-applicable action reappearing) would go unnoticed. Add a small test for types.ts covering hidden/non-hidden leaves and a hidden parent.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| return actions.flatMap((action) => { | ||
| // If label is empty, hide | ||
| if (!action.label) { | ||
| if (!action.label || action.hidden) { |
Contributor
There was a problem hiding this comment.
P3: The new hidden filtering in flattenActions changes which actions appear in the command palette for all cell, notebook and config actions, with non-obvious subtleties: a hidden parent drops its whole subtree (e.g. "Share" when no sharing option is enabled) while redundant parents still recurse. None of this is covered by a unit test — flattenActions has no test file — so a regression here (e.g. a hidden parent silently removing runnable children, or a non-applicable action reappearing) would go unnoticed. Add a small test for types.ts covering hidden/non-hidden leaves and a hidden parent.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/components/editor/actions/types.ts, line 48:
<comment>The new `hidden` filtering in flattenActions changes which actions appear in the command palette for all cell, notebook and config actions, with non-obvious subtleties: a hidden parent drops its whole subtree (e.g. "Share" when no sharing option is enabled) while `redundant` parents still recurse. None of this is covered by a unit test — flattenActions has no test file — so a regression here (e.g. a hidden parent silently removing runnable children, or a non-applicable action reappearing) would go unnoticed. Add a small test for types.ts covering hidden/non-hidden leaves and a hidden parent.</comment>
<file context>
@@ -45,8 +45,7 @@ export function flattenActions(
return actions.flatMap((action) => {
- // If label is empty, hide
- if (!action.label) {
+ if (!action.label || action.hidden) {
return [];
}
</file context>
peter-gy
approved these changes
Aug 23, 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
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.
The export dialog contains every notebook download format, so the separate submenu repeats the same choices. The command palette keeps the direct format actions.