Skip to content
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

feat!: Added support for separators in menus. #8767

Merged
merged 3 commits into from
Feb 27, 2025

Conversation

gonfunko
Copy link
Contributor

@gonfunko gonfunko commented Feb 13, 2025

The basics

The details

Resolves

Fixes #8752

Proposed Changes

This PR adds support for separators in all Blockly menus, including contextual menus on the workspace, blocks and comments, and in dropdown menus spawned from FieldDropdown.

For contextual menus, separators can be added via the context menu registry in a similar manner to normal menu items:

  const separator: RegistryItem = {
    separator: true,
    scopeType: ContextMenuRegistry.ScopeType.WORKSPACE,
    id: 'whatever-id-goes-here',
    weight: 2,
  };
  ContextMenuRegistry.registry.register(separator);

For FieldDropdown, separators can be added via JSON (or a generator function) by using the string literal separator:

{
  'type': 'field_dropdown',
  'name': 'SOME_DROPDOWN_FIELD',
  'options': [
    ['Normal menu item', 'normal'],
    ['Other menu item', 'other'],
    'separator',
    ['Different menu item', 'different'],
  ],
},

Breaking changes

In general existing code should continue to work, but in order to accomodate typings ContextMenuOption and RegistryItem are now types instead of interfaces. Core and our documentation suggest using these as object literals, as in the example above in this PR description, which will continue to work as-is, but if users have a class that implements either of these interfaces, the implements will need to be removed.

@gonfunko gonfunko requested a review from a team as a code owner February 13, 2025 23:18
@github-actions github-actions bot added breaking change Used to mark a PR or issue that changes our public APIs. PR: feature Adds a feature and removed PR: feature Adds a feature breaking change Used to mark a PR or issue that changes our public APIs. labels Feb 13, 2025
);
} else if (typeof tuple[1] !== 'string') {
} else if (typeof option[1] !== 'string') {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't this fail if the option is just FieldDropdown.SEPARATOR instead of a tuple?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No; option[1] would be "e" in that case, since option would be the string "separator", and the type of "e" is still string.

@@ -35,14 +36,10 @@ import {Svg} from './utils/svg.js';
* Class for an editable dropdown field.
*/
export class FieldDropdown extends Field<string> {
/** Horizontal distance that a checkmark overhangs the dropdown. */
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why remove these two?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They're unreferenced in the entire codebase. I suppose this is technically a breaking change, but they feel very implementation detail-y. Happy to revert if you want though.

@gonfunko gonfunko merged commit fa4fce5 into google:rc/v12.0.0 Feb 27, 2025
7 checks passed
@gonfunko gonfunko deleted the menu-separator branch February 27, 2025 22:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Used to mark a PR or issue that changes our public APIs. PR: feature Adds a feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants