feat: add MenuConfiguration #20138

tltv · 2024-10-03T14:14:41Z

New public API for building application menu: adds MenuConfiguration, MenuOptions and MenuOption where MenuConfiguration is the main entry point to access menu data to build main menu.

Fixes: #20063

github-actions · 2024-10-03T14:47:44Z

Test Results

1 140 files + 1 1 140 suites +1 1h 8m 39s ⏱️ -23s
7 422 tests + 3 7 372 ✅ + 3 50 💤 ±0 0 ❌ ±0
7 706 runs - 15 7 648 ✅ - 16 58 💤 +1 0 ❌ ±0

Results for commit 95ac4a7. ± Comparison against base commit aa83939.

♻️ This comment has been updated with latest results.

tltv · 2024-10-04T07:17:50Z

@Artur- do you have any feedback regarding this new public Menu API called MenuConfiguration. You updated Start app with MenuRegisty, so this new API would be a replacement for MenuRegistry. E.g. instead of calling MenuRegistry.collectMenuItemsList() you would call MenuConfiguration.getMenuOptions().get() which returns List<MenuOption>.

Artur- · 2024-10-04T07:44:49Z

Is there a reason I should call MenuConfiguration.getMenuOptions().get() instead of MenuConfiguration.getMenuOptions() to get the menu options?

tltv · 2024-10-04T08:11:42Z

Is there a reason I should call MenuConfiguration.getMenuOptions().get() instead of MenuConfiguration.getMenuOptions() to get the menu options?

If you don't see reason, we might not need to do it this way. What do you think if MenuConfiguration.getMenuOptions() would just return List<MenuOption> directly? And especially what do you think of MenuOptions class in general, does it look like useful for developers or could we just go with a List instead?

I'm not so sure how useful MenuOptions would be. Also pinging @caalador for more input.

caalador · 2024-10-04T08:19:02Z

From the writing of the ticket the target was to get List<MenuOptions> which would be in the implemented case the same as List<MenuOption> (Options was noting that it contains multiple parameters instead of only being an Option)

tltv · 2024-10-04T08:38:14Z

From the writing of the ticket the target was to get List<MenuOptions> which would be in the implemented case the same as List<MenuOption> (Options was noting that it contains multiple parameters instead of only being an Option)

And MenuOption name was picked because MenuItem was already reserved by components. Let's wait if @mshabarov had some idea with splitting in MenuOptions and MenuOption in ticket requirements. Maybe we can remove MenuOptions. Naming properly is still a bit challenging here.

mshabarov · 2024-10-04T09:07:40Z

I was thinking that MenuOptions class could have List<MenuOption> and some other fields that are common to menu. However, I don't know what would this be and, if we need to expose it, we can add a new method in MenuConfiguration.
Additionally, the code that renders menu should be as simple as

MenuConfiguration.getMenuOptions().forEach(option -> new SideNavItem(option.getTitle(), option.getRoute()));

Thus, I agree that we could simplify it to have just a collection of MenuOption.

mshabarov · 2024-10-04T09:21:05Z

Possible alternatives are MenuEntry, MenuAction, MenuChoice, MenuNode,MenuCommand, MenuLinkData.

How about MenuEntry ?

tltv · 2024-10-04T09:31:07Z

How about MenuEntry ?

MenuEntry is also vacant. +1 from me.

mshabarov · 2024-10-04T09:39:41Z

flow-server/src/main/java/com/vaadin/flow/server/menu/MenuConfiguration.java

+     *
+     * @return the {@link MenuOptions} instance
+     */
+    public static MenuOptions getMenuOptions() {


this probably should expose the same method, but with Locale (to allows alphabetical order based on exact translation)

mshabarov · 2024-10-04T09:40:31Z

flow-server/src/main/java/com/vaadin/flow/server/menu/MenuConfiguration.java

+     *
+     * @return the {@link MenuOptions} instance
+     */
+    public static MenuOptions getMenuOptions() {


I don't see MenuRegistry javadocs update, these are needed to highlight it's internal use.

mshabarov · 2024-10-04T09:44:44Z

flow-server/src/main/java/com/vaadin/flow/server/menu/MenuOption.java

+ *            path to the icon. E.g. 'line-awesome/svg/lock-open-solid.svg'.
+ * @param menuClass
+ *            the source class with {@link com.vaadin.flow.router.Menu}
+ *            annotation or null if not available


Worth to mention that for menu entries pointing to Hilla/TypeScript views, this will be null

mshabarov · 2024-10-04T10:06:45Z

flow-server/src/main/java/com/vaadin/flow/router/MenuData.java

 /**
 * Data class for menu item information.
 * <p>
 * Only for read as data is immutable.
 */
-public record MenuData(String title, Double order, boolean exclude, String icon) implements Serializable {
+public record MenuData(String title, Double order, boolean exclude, String icon, Class<? extends Component> menuClass) implements Serializable {


This should be deprecated and marked for internal use. We can't delete it as it's already shipped with 24.4, but we either shouldn't encourage it to be used in public API.

Good point. Someone might use it already with 24.4 so need to keep and deprecate old. But it's also part of the public RouteData API which is not internal API and it's in 24.4. Marking it internal now is not possible afaik.

Hmm, okay, let's keep it as is then.

mshabarov · 2024-10-04T10:08:30Z

flow-server/src/main/java/com/vaadin/flow/server/menu/MenuConfiguration.java

+
+/**
+ * Menu configuration helper class to retrieve available menu entries for
+ * application main menu.


Add "since 24.5" note

mshabarov · 2024-10-04T10:41:54Z

flow-server/src/main/java/com/vaadin/flow/server/menu/MenuConfiguration.java

+ *
+ * @since 24.5
+ */
+public class MenuConfiguration {


nit: this can be final class

New public API for building application menu: adds `MenuConfiguration`, `MenuOptions` and `MenuOption` where `MenuConfiguration` is the main entry point to access menu data to build main menu. Fixes: #20063

Renamed MenuOption to MenuEntry.

Removed Serializable from MenuConfiguration.

sonarcloud · 2024-10-04T10:58:01Z

Quality Gate passed

Issues
17 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

tltv · 2024-10-04T11:05:35Z

Drafted PR that updates imports in Hilla: vaadin/hilla#2794
After this one is merged, that one should be merged too.

* feat: add MenuConfiguration New public API for building application menu: adds `MenuConfiguration`, `MenuOptions` and `MenuOption` where `MenuConfiguration` is the main entry point to access menu data to build main menu. Fixes: #20063 * chore: renamed classes and removed MenuOptions Renamed MenuOption to MenuEntry. * chore: updated javadocs * chore: moved MenuRegistry to internal package Removed Serializable from MenuConfiguration. * chore: added javadoc and deprecated MenuData constructor * chore: use new constructor * chore: make MenuConfiguration final

* feat: add MenuConfiguration New public API for building application menu: adds `MenuConfiguration`, `MenuOptions` and `MenuOption` where `MenuConfiguration` is the main entry point to access menu data to build main menu. Fixes: #20063 * chore: renamed classes and removed MenuOptions Renamed MenuOption to MenuEntry. * chore: updated javadocs * chore: moved MenuRegistry to internal package Removed Serializable from MenuConfiguration. * chore: added javadoc and deprecated MenuData constructor * chore: use new constructor * chore: make MenuConfiguration final Co-authored-by: Tomi Virtanen <[email protected]>

vaadin-bot added the +1.0.0 label Oct 3, 2024

tltv force-pushed the feat/20063-menu-configuration branch from 0042e6b to 42cc7ce Compare October 3, 2024 14:31

tltv force-pushed the feat/20063-menu-configuration branch from 42cc7ce to 4ff26d4 Compare October 4, 2024 07:05

mshabarov self-requested a review October 4, 2024 07:15

mshabarov requested changes Oct 4, 2024

View reviewed changes

mshabarov reviewed Oct 4, 2024

View reviewed changes

mshabarov requested changes Oct 4, 2024

View reviewed changes

mshabarov reviewed Oct 4, 2024

View reviewed changes

mshabarov previously approved these changes Oct 4, 2024

View reviewed changes

tltv added 6 commits October 4, 2024 13:42

feat: add MenuConfiguration

98e8810

New public API for building application menu: adds `MenuConfiguration`, `MenuOptions` and `MenuOption` where `MenuConfiguration` is the main entry point to access menu data to build main menu. Fixes: #20063

chore: renamed classes and removed MenuOptions

3b106bd

Renamed MenuOption to MenuEntry.

chore: updated javadocs

9a1a008

chore: moved MenuRegistry to internal package

6693ec4

Removed Serializable from MenuConfiguration.

chore: added javadoc and deprecated MenuData constructor

afeab72

chore: use new constructor

920c818

tltv dismissed mshabarov’s stale review via 920c818 October 4, 2024 10:46

tltv force-pushed the feat/20063-menu-configuration branch from bb3fa46 to 920c818 Compare October 4, 2024 10:46

chore: make MenuConfiguration final

95ac4a7

mshabarov approved these changes Oct 4, 2024

View reviewed changes

tltv marked this pull request as ready for review October 4, 2024 11:05

mshabarov added the target/24.5 label Oct 4, 2024

mshabarov merged commit 4cf08cf into main Oct 4, 2024
26 checks passed

mshabarov deleted the feat/20063-menu-configuration branch October 4, 2024 11:29

vaadin-bot added the cherry-picked-24.5 label Oct 4, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add MenuConfiguration #20138

feat: add MenuConfiguration #20138

tltv commented Oct 3, 2024

github-actions bot commented Oct 3, 2024 •

edited

Loading

tltv commented Oct 4, 2024

Artur- commented Oct 4, 2024

tltv commented Oct 4, 2024

caalador commented Oct 4, 2024

tltv commented Oct 4, 2024

mshabarov commented Oct 4, 2024

mshabarov commented Oct 4, 2024

tltv commented Oct 4, 2024

mshabarov Oct 4, 2024 •

edited

Loading

mshabarov Oct 4, 2024

mshabarov Oct 4, 2024

mshabarov Oct 4, 2024

tltv Oct 4, 2024

mshabarov Oct 4, 2024

mshabarov Oct 4, 2024

mshabarov Oct 4, 2024

sonarcloud bot commented Oct 4, 2024

tltv commented Oct 4, 2024

feat: add MenuConfiguration #20138

feat: add MenuConfiguration #20138

Conversation

tltv commented Oct 3, 2024

github-actions bot commented Oct 3, 2024 • edited Loading

Test Results

tltv commented Oct 4, 2024

Artur- commented Oct 4, 2024

tltv commented Oct 4, 2024

caalador commented Oct 4, 2024

tltv commented Oct 4, 2024

mshabarov commented Oct 4, 2024

mshabarov commented Oct 4, 2024

tltv commented Oct 4, 2024

mshabarov Oct 4, 2024 • edited Loading

Choose a reason for hiding this comment

mshabarov Oct 4, 2024

Choose a reason for hiding this comment

mshabarov Oct 4, 2024

Choose a reason for hiding this comment

mshabarov Oct 4, 2024

Choose a reason for hiding this comment

tltv Oct 4, 2024

Choose a reason for hiding this comment

mshabarov Oct 4, 2024

Choose a reason for hiding this comment

mshabarov Oct 4, 2024

Choose a reason for hiding this comment

mshabarov Oct 4, 2024

Choose a reason for hiding this comment

sonarcloud bot commented Oct 4, 2024

Quality Gate passed

tltv commented Oct 4, 2024

github-actions bot commented Oct 3, 2024 •

edited

Loading

mshabarov Oct 4, 2024 •

edited

Loading