lukasbach / react-complex-tree

Unopinionated Accessible Tree Component with Multi-Select and Drag-And-Drop
https://rct.lukasbach.com
MIT License
982 stars 82 forks source link

Non-contiguous multi-selection doesn't work with `cmd` + click on Mac #80

Closed tonyketcham closed 2 years ago

tonyketcham commented 2 years ago

Describe the bug On a Mac, attempting to select multiple nodes while holding down cmd + clicking does not work. It seems that the Windows ctrl key may be mapped to the mac control key rather than the expected cmd key.

Holding down control + click on a Mac performs a right-click on the system level which would overpower the tree's hotkey if it is indeed using control + click.

The only way I can select a non-contiguous set of nodes on Mac is via the keyboard when I do control + space.

To Reproduce Attempt to cmd + click to perform a non-contiguous multi-select while using a Mac.

Expected behavior cmd + click should have the same effect as control + space currently does

Screenshots Here's attempting to do this via mouse. First is holding down cmd while clicking, second is holding control while clicking

https://user-images.githubusercontent.com/43280336/171761705-fec53f3e-a0d5-445f-8337-b496f181258b.mov

Desktop (please complete the following information):

alexdarling commented 2 years ago

Running into the same issue on Catalina.

I think this can be fixed by modifying a handful of spots that look for e.ctrlKey so they also look for e.metaKey. This has the knock-on effect of allowing Windows users to use the Windows key for multi-selection, which doesn't seem like a big deal, but might not be desirable?

tonyketcham commented 2 years ago

@alexdarling those places can be replaced with a dynamic keyboard shortcut that gets set based on the user's OS via something like:

import { detect } from 'detect-browser';

export const OS = browser.os;
export const isMacOS = OS === 'Mac OS';

// Handle device differences in keyboard key names
export const CTRL_CMD = isMacOS ? 'cmd' : 'ctrl';

This way it wouldn't introduce any side effects for Windows

lukasbach commented 2 years ago

Thank you for your report, and everyone who suggested ideas for fixing this! This is fixed in version 1.1.9.

selfrefactor commented 2 years ago

I think this re-emerged with 1.1.11. I cannot test it out as I am not on Mac, but I received report that issue is there with latest library version. In other words, it works with 1.1.10 but not the latest.

lukasbach commented 2 years ago

@selfrefactor hm that is odd. There weren't really any changes between 1.1.10 and 1.1.11 that had anything to do with that. I would be interested if someone with a Mac can reproduce this, unfortunately I also do not have a mac. Can you open a new issue for that?