Summary

Add a model and collections for tracking menu items.

Review Request #13630 — Created March 13, 2024 and submitted March 18, 2024, 10:55 p.m.

Information

Owner

chipx86

Repository

Ink

Branch

master

Bugs

Depends On

Reviewers

Groups

beanbag-misc

People

Description

This introduces the model-level support for menus, consisting of three
parts:

MenuItem: This is a model containing state for any kind of supported
menu item (a standard action item, checkbox menu item, radio menu
item, or a separator).
MenuItemsCollection: A collection of menu items, which will be used
as the backing for a menu view.
MenuItemsRadioGroup: A radio group for tracking menu items, which
ensures only one item can be marked as checked at a time.

The design for MenuItem consolidates all state for all types into one
model, rather than having subclasses for each type. There are a few
reasons for this:

It makes it easier to maintain items in a collection and still allow
instantiation of new entries via the collection's .add().
It avoids needing to do instanceof checks for different operations.
It seems to be common practice (looking at the Apple, Windows, and
GTK foundational classes for menu items).

The upcoming MenuView will make use of these classes to provide a full
menu implementation.

Testing Done

Unit tests pass.

Tested this thoroughly with the in-progress MenuView work.

Commits

Summary	ID
Add a model and collections for tracking menu items. This introduces the model-level support for menus, consisting of three parts: * `MenuItem`: This is a model containing state for any kind of supported menu item (a standard action item, checkbox menu item, radio menu item, or a separator). * `MenuItemsCollection`: A collection of menu items, which will be used as the backing for a menu view. * `MenuItemsRadioGroup`: A radio group for tracking menu items, which ensures only one item can be marked as checked at a time. The design for `MenuItem` consolidates all state for all types into one model, rather than having subclasses for each type. There are a few reasons for this: 1. It makes it easier to maintain items in a collection and still allow instantiation of new entries via the collection's `.add()`. 2. It avoids needing to do `instanceof` checks for different operations. 3. It seems to be common practice (looking at the Apple, Windows, and GTK foundational classes for menu items). The upcoming `MenuView` will make use of these classes to provide a full menu implementation.	170360f5af6c75c26e4f86272597314bb89e1b68

Summary

Add a model and collections for tracking menu items.

This introduces the model-level support for menus, consisting of three parts: * `MenuItem`: This is a model containing state for any kind of supported menu item (a standard action item, checkbox menu item, radio menu item, or a separator). * `MenuItemsCollection`: A collection of menu items, which will be used as the backing for a menu view. * `MenuItemsRadioGroup`: A radio group for tracking menu items, which ensures only one item can be marked as checked at a time. The design for `MenuItem` consolidates all state for all types into one model, rather than having subclasses for each type. There are a few reasons for this: 1. It makes it easier to maintain items in a collection and still allow instantiation of new entries via the collection's `.add()`. 2. It avoids needing to do `instanceof` checks for different operations. 3. It seems to be common practice (looking at the Apple, Windows, and GTK foundational classes for menu items). The upcoming `MenuView` will make use of these classes to provide a full menu implementation.

170360f5af6c75c26e4f86272597314bb89e1b68

Issues

Description	From	Last Updated
Needs a trailing comma.	david	March 17, 2024, 1:03 a.m.
Can you add a module comment to the top of the file?	david	March 17, 2024, 1:04 a.m.
Can you add a module comment to the top of the file?	david	March 17, 2024, 1:04 a.m.

flake8 passed.

JSHint passed.

src/ink/js/components/collections/menuItemsRadioGroup.ts (Diff revision 1)
The issue has been dropped. Show all issues
```
Needs a trailing comma.
```
1. chipx86 March 17, 2024, 1:04 a.m.
  It's not allowed for ...args. TypeScript and ESLint will complain.
src/ink/js/components/collections/tests/menuItemsRadioGroupTests.ts (Diff revision 1)
The issue has been resolved. Show all issues
```
Can you add a module comment to the top of the file?
```
src/ink/js/components/models/tests/menuItemModelTests.ts (Diff revision 1)
The issue has been resolved. Show all issues
```
Can you add a module comment to the top of the file?
```

Change Summary:


change:checked signals are now always triggered, and are never silenced, letting UIs properly update their state when unchecking.
Added module-level docstrings for unit tests.

Commits:

	Summary	ID
	Add a model and collections for tracking menu items. This introduces the model-level support for menus, consisting of three parts: * `MenuItem`: This is a model containing state for any kind of supported menu item (a standard action item, checkbox menu item, radio menu item, or a separator). * `MenuItemsCollection`: A collection of menu items, which will be used as the backing for a menu view. * `MenuItemsRadioGroup`: A radio group for tracking menu items, which ensures only one item can be marked as checked at a time. The design for `MenuItem` consolidates all state for all types into one model, rather than having subclasses for each type. There are a few reasons for this: 1. It makes it easier to maintain items in a collection and still allow instantiation of new entries via the collection's `.add()`. 2. It avoids needing to do `instanceof` checks for different operations. 3. It seems to be common practice (looking at the Apple, Windows, and GTK foundational classes for menu items). The upcoming `MenuView` will make use of these classes to provide a full menu implementation.	5013052b5e761d4cc6a337c1c96d01ed79558452
	Add a model and collections for tracking menu items. This introduces the model-level support for menus, consisting of three parts: * `MenuItem`: This is a model containing state for any kind of supported menu item (a standard action item, checkbox menu item, radio menu item, or a separator). * `MenuItemsCollection`: A collection of menu items, which will be used as the backing for a menu view. * `MenuItemsRadioGroup`: A radio group for tracking menu items, which ensures only one item can be marked as checked at a time. The design for `MenuItem` consolidates all state for all types into one model, rather than having subclasses for each type. There are a few reasons for this: 1. It makes it easier to maintain items in a collection and still allow instantiation of new entries via the collection's `.add()`. 2. It avoids needing to do `instanceof` checks for different operations. 3. It seems to be common practice (looking at the Apple, Windows, and GTK foundational classes for menu items). The upcoming `MenuView` will make use of these classes to provide a full menu implementation.	4597d7dbca74ed04e283e25433c33014f19a6e33

Diff:

Revision 2 (+2354 -4)

Show changes

	src/ink/js/components/collections/index.ts
	src/ink/js/components/collections/menuItemsCollection.ts
	src/ink/js/components/collections/menuItemsRadioGroup.ts
	src/ink/js/components/collections/tests/menuItemsRadioGroupTests.ts
	src/ink/js/components/models/index.ts
	src/ink/js/components/models/menuItemModel.ts
	src/ink/js/components/models/tests/menuItemModelTests.ts

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Change Summary:

Actually updated with the new changes.

Commits:

	Summary	ID
	Add a model and collections for tracking menu items. This introduces the model-level support for menus, consisting of three parts: * `MenuItem`: This is a model containing state for any kind of supported menu item (a standard action item, checkbox menu item, radio menu item, or a separator). * `MenuItemsCollection`: A collection of menu items, which will be used as the backing for a menu view. * `MenuItemsRadioGroup`: A radio group for tracking menu items, which ensures only one item can be marked as checked at a time. The design for `MenuItem` consolidates all state for all types into one model, rather than having subclasses for each type. There are a few reasons for this: 1. It makes it easier to maintain items in a collection and still allow instantiation of new entries via the collection's `.add()`. 2. It avoids needing to do `instanceof` checks for different operations. 3. It seems to be common practice (looking at the Apple, Windows, and GTK foundational classes for menu items). The upcoming `MenuView` will make use of these classes to provide a full menu implementation.	4597d7dbca74ed04e283e25433c33014f19a6e33
	Add a model and collections for tracking menu items. This introduces the model-level support for menus, consisting of three parts: * `MenuItem`: This is a model containing state for any kind of supported menu item (a standard action item, checkbox menu item, radio menu item, or a separator). * `MenuItemsCollection`: A collection of menu items, which will be used as the backing for a menu view. * `MenuItemsRadioGroup`: A radio group for tracking menu items, which ensures only one item can be marked as checked at a time. The design for `MenuItem` consolidates all state for all types into one model, rather than having subclasses for each type. There are a few reasons for this: 1. It makes it easier to maintain items in a collection and still allow instantiation of new entries via the collection's `.add()`. 2. It avoids needing to do `instanceof` checks for different operations. 3. It seems to be common practice (looking at the Apple, Windows, and GTK foundational classes for menu items). The upcoming `MenuView` will make use of these classes to provide a full menu implementation.	170360f5af6c75c26e4f86272597314bb89e1b68

Diff:

Revision 3 (+2382 -4)

Show changes

	src/ink/js/components/collections/index.ts
	src/ink/js/components/collections/menuItemsCollection.ts
	src/ink/js/components/collections/menuItemsRadioGroup.ts
	src/ink/js/components/collections/tests/menuItemsRadioGroupTests.ts
	src/ink/js/components/models/index.ts
	src/ink/js/components/models/menuItemModel.ts
	src/ink/js/components/models/tests/menuItemModelTests.ts

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Status: Closed (submitted)

Change Summary:

Pushed to master (c50b9d1)