-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
Desktop: Resolves #1556: Support selecting multiple notebooks #13612
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Desktop: Resolves #1556: Support selecting multiple notebooks #13612
Conversation
|
Will this add support for exporting a specific selection of notebooks? |
Not currently, but that would make sense to add (in particular because a user might expect right-click > export to export all selected notebooks). At present, it supports:
Other actions (e.g. sharing notebooks, exporting) use the primary selected notebook. Edit: For now, the "Export" right-click menu is hidden when multiple notebooks are selected. If implemented, exporting multiple notebooks will probably be done in a separate pull request. |
Certain test mocks pass only partial state to stateToWhenClauseContext. This commit updates stateToWhenClauseContext to handle this case gracefully.
… hidden when disabled
| // All context folders | ||
| foldersAreDeleted: commandFolders.every(f => !!f.deleted_time), | ||
| foldersIncludeReadOnly: commandFolders.some(f => itemIsReadOnlySync(ModelType.Folder, ItemChange.SOURCE_UNSPECIFIED, f as ItemSlice, settings['sync.userId'], state.shareService)), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New state variables that are computed from all command folders, rather than just one, have been added. Previous variables (e.g. folderIsTrash) continue to apply to the main selected/command folder.
| public commandToStatefulMenuItem(commandName: string, ...args: any[]): MenuItem { | ||
| const whenClauseContext = this.service.currentWhenClauseContext(); | ||
| public commandToStatefulMenuItem(commandName: string, commandTarget?: any, options?: WhenClauseContextOptions): MenuItem { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refactoring: commandToStatefulMenuItem now 1) forwards only one argument to the target command, 2) allows specifying the folder/note that the command applies to for the enabled/disabled state:
- All uses of
commandToStatefulMenuItem(code search) pass at most one argument to the command. - This allows creating a
whenClauseContextfor the items the command will act on, rather than the selection.- Basing the when clause context on the selection can be problematic if 1) the menu is being shown for an unselected folder, and/or 2) if the folder for the menu is in
selectedFolderIds, but the selection isn't shown in the UI because a different item type is selected (e.g. a tag).
- Basing the when clause context on the selection can be problematic if 1) the menu is being shown for an unselected folder, and/or 2) if the folder for the menu is in
For example, this allows disabling the menu item for the deleteFolder command when one of the target folders is read-only, rather than basing this on the selection. (With this PR, it's still possible to right-click > delete unselected folders).
An alternative would be to refactor folderCommandToMenuItem (a new function in useOnRenderItem) to call commandToMenuItem directly.
| className={`list-item-container ${selected ? 'selected' : ''}`} | ||
| highlightOnHover={true} | ||
| onDrop={props.onTagDrop} | ||
| onContextMenu={props.onContextMenu} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved onContextMenu: This is for consistency with the FolderItem component and allows the context menu event handler to access HTML properties defined by ListItemWrapper, including data-index. The onContextMenu event handler currently determines the source note/tag based on event.currentTarget.
Summary
This pull request adds support for selecting multiple notebooks (or tags) in the sidebar on desktop.
Resolves #1556.
Notes
Currently supports:
Limitations
Fewer actions are available in the right-click menu when multiple notebooks are selected:
Confusing behavior: Selecting multiple notebooks or tags only shows items associated with one of the notebooks/tags in the notes list. In particular, only notes associated with the first selected notebook/tag are shown in the notes list.
Confusing behavior: Attempting to select both multiple notebooks and tags at the same time can lead to confusing behavior:
Screencast.from.2025-11-04.15-43-33.webm
Above, pressing ctrl-click to attempt to add a tag to the selection only visually hides the notebook selection. If a notebook is then added to the selection (again with ctrl-click), the notebook selection becomes visible again, with the notebook added with ctrl-click.
Screen recording
Screencast.from.2025-11-04.15-23-08.webm
The above screen recording demonstrates:
Testing
In addition to the manual testing demonstrated by the screen recording above and the automated tests added by this pull request, I've verified that it's still possible to: