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

Fix a bug where pressing a Ctrl/Cmd does not trigger non-contiguous selection #10502

Merged
merged 6 commits into from Sep 13, 2023

Conversation

budnix
Copy link
Member

@budnix budnix commented Sep 6, 2023

Context

The PR fixes a bug where pressing a Ctrl/Cmd does not trigger a non-contiguous selection. The fix changes the behavior of how the state (pressed or not) of the modifiers' keys (Ctrl/Cmd, Shift, etc.) are observed. Previously, they were observed only when at least one of the Handsontable on the page was activated (by click or setListening method call). Now, they are observed once, globally, right after the table is initialized, whether the table is active or not.

How has this been tested?

I tested the changes locally, and I covered the fix by update tests.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Related issue(s):

  1. fixes https://github.com/handsontable/dev-handsontable/issues/1025

Affected project(s):

  • handsontable

Checklist:

@budnix budnix self-assigned this Sep 6, 2023
@budnix budnix marked this pull request as ready for review September 6, 2023 12:25
@@ -4713,11 +4713,8 @@ export default function Core(rootElement, userSettings, rootInstanceSymbol = fal
};

const shortcutManager = createShortcutManager({
handleEvent(event) {
const isListening = instance.isListening();
const isKeyboardEventWithKey = event?.key !== void 0;
Copy link
Member Author

Choose a reason for hiding this comment

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

@wszymanski I removed this line as I felt it's here only for tests, not for real user use cases. The event object is always passed as it is directly passed from the native event, and looking through the MDN and typing the key property seems to have always some value.

Copy link
Contributor

Choose a reason for hiding this comment

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

I can't find a reason why it's there.

@github-actions
Copy link

github-actions bot commented Sep 8, 2023

Launch the local version of documentation by running:

npm run docs:review 77b32da2be2d27756f1c0e329dea01ae6ff728a9

@@ -4713,11 +4713,8 @@ export default function Core(rootElement, userSettings, rootInstanceSymbol = fal
};

const shortcutManager = createShortcutManager({
handleEvent(event) {
const isListening = instance.isListening();
const isKeyboardEventWithKey = event?.key !== void 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

I can't find a reason why it's there.

@budnix budnix merged commit a262af5 into develop Sep 13, 2023
21 checks passed
@budnix budnix deleted the feature/dev-issue-1025 branch September 13, 2023 08:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants