vaadin / flow-components

Java counterpart of Vaadin Web Components
100 stars 66 forks source link

Menu bar should support click() function #3519

Open timbo86 opened 2 years ago

timbo86 commented 2 years ago

Description

When having a component column inside a grid with only a menu bar inside, focusing any cell of this column via keyboard and pressing spacebar executes default grid behaviour of selecting whole row.

Expected outcome

I would expect the menu bar to behave like a button and trigger click event on spacebar key press. At least if there is only one button or only the overflow button is visible (in this case it should simply open the overlay)

Minimal reproducible example

Create following view:

@Route("grid-button")
public class GridButtonView extends Div {

    public GridButtonView() {
        Grid<User> grid = new Grid<>();
        grid.addColumn(User::getFirstName);
        grid.addColumn(User::getLastName);

        grid.addComponentColumn(u -> new Button(
            VaadinIcon.USER.create(),
            e -> new Notification(String.format("User %s clicked", u.getLastName())).setOpened(true)));
        grid.addComponentColumn(u -> {
            MenuBar menu = new MenuBar();
            menu.addItem(VaadinIcon.USER.create(), e -> new Notification(String.format("User %s clicked", u.getLastName())).setOpened(true));
            return menu;
        });

        grid.setItems(new User("First", "User1"), new User("Second", "User2"), new User("Third", "User3"), new User("Fourth", "User4"));

        add(grid);
    }

    private static class User {

        private String firstName;
        private String lastName;

        public User(String firstName, String lastName) {
            this.firstName = firstName;
            this.lastName = lastName;
        }

        public String getLastName() {
            return lastName;
        }

        public String getFirstName() {
            return firstName;
        }

    }

}

Steps to reproduce

Tab via keyboard into the grid body. Then navigate to any button component cell (third column) with arrow keys, then press spacebar. => The click event is triggered correctly and a notification is shown.

Navigate to next column cell and press spacebar. => The click event is not triggered. Whole row is selected/deselected

Environment

Vaadin version(s): 23.1.2

Browsers

Chrome

TatuLund commented 2 years ago

Could it be so, that the culprit is that when you tab to that cell, the focus does not transfer to MenuBar and therefore when press space, it is just Grid taking that keypress, which naturally toggles the selection?

timbo86 commented 2 years ago

Check out vaadin-grid-keyboard-navigation-mixin.js on key up: image

When I create my own component inheriting from menu bar and doing something like this:

click() {
    if (this._hasOverflow) {
        this.shadowRoot.querySelector('[part="overflow-button"]').dispatchEvent(new Event("click", {bubbles: true}));
    }
}

Of course this will only open menu of overflow button, but for now this is our only use case and it works.

web-padawan commented 2 years ago

In Vaadin 23.2 alpha we added the has-single-button attribute to vaadin-menu-bar, see https://github.com/vaadin/web-components/pull/4175

So the click() function in vaadin-menu-bar in this case could be modified as follows:

click() {
  if (this.hasAttribute('has-single-button')) {
    const overflow = this._overflow;
    const button = overflow.hasAttribute('hidden') ? this._buttons[0] : overflow;
    overflow.click();
  }
}

I'm not quite sure we should support this out of the box, as generally MenuBar was not designed to be used inside Grid.

timbo86 commented 2 years ago

That would be exactly what we would need.

Using a menu bar with only the overflow button in a grid has the advantage of beeing able to add multiple actions next to lots of text columns and the grid would still not look overloaded.