fix: check if keydown event is prevent before closing

[DiegoCardoso]

Description

Overlay allows the vaadin-overlay-escape-press custom to be default prevented before closing. This change also allows the defaultPrevent called on the keydown event to stop the overlay from closing.

That should allow cases where, while interacting with an interactive element inside a Grid cell, hitting Esc to move the focus back to the cell, the overlay parent won't be closed, since Grid calls preventDefault on the keydown event.

To manually test it, add this snippet to the /dev/dialog.html page:

        import '@vaadin/grid';
        //...

        const grid = document.createElement('vaadin-grid');
        grid.innerHTML = `<vaadin-grid-column id="buttonColumn"></vaadin-grid-column>`;
        grid.items = [{ name: 'foo' }, { name: 'bar' }];
        container.appendChild(grid);
        const buttonColumn = grid.querySelector('#buttonColumn');
        buttonColumn.renderer = (root, _, model) => {
          if (root.firstElementChild) return;

          const button = document.createElement('button');
          button.textContent = `Button for ${model.item.name}`;
          root.appendChild(button);
        };

Fixes vaadin/flow-components#8176

Type of change

• Bugfix

[DiegoCardoso]

With the latest change, some tests in DatePicker started to fail. preventDefault is being called here:

web-components/packages/date-picker/src/vaadin-date-picker-mixin.js

Lines 1083 to 1086 in 1a10749

	if (allowedKeys.indexOf(e.keyCode) === -1) {
	e.preventDefault();
	}
	}

... for all keys, except Tab. I wasn't able to fully understand the reason from reading the comment, and I wonder if it's safe to whitelist Escape as well.

[DiegoCardoso]

With the latest change, some tests in DatePicker started to fail. preventDefault is being called here:

web-components/packages/date-picker/src/vaadin-date-picker-mixin.js

Lines 1083 to 1086 in 1a10749

	if (allowedKeys.indexOf(e.keyCode) === -1) {
	e.preventDefault();
	}
	}

... for all keys, except Tab. I wasn't able to fully understand the reason from reading the comment, and I wonder if it's safe to whitelist Escape as well.

Sincerely, I don't know what this call is actually doing. Removing it doesn't affect the tests in DatePicker.

[web-padawan]

I would suggest to also add an integration test e.g. dialog-grid.test.js similar to dialog-date-picker.test.js.

[DiegoCardoso]

I would suggest to also add an integration test e.g. dialog-grid.test.js similar to dialog-date-picker.test.js.

Done.

[web-padawan]

LGTM. Improved the test to use sendKeys() which we usually do for integration tests.

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

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

See analysis details on SonarQube Cloud