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

LO captures cmd-opt-<arrow> #151

Closed
kdawson opened this issue Feb 2, 2020 · 14 comments
Closed

LO captures cmd-opt-<arrow> #151

kdawson opened this issue Feb 2, 2020 · 14 comments

Comments

@kdawson
Copy link

@kdawson kdawson commented Feb 2, 2020

I'm a new user, so no idea if this behavior is old or new; or whether it is intended. I map cmd-opt-{left or right}-arrow in all browsers to navigate among open tabs. In Structure Mode LO captures these keystrokes and maps them simply to {l-or-r}-arrow, and executes flatdown or flatup.

@scripting

This comment has been minimized.

Copy link
Owner

@scripting scripting commented Feb 3, 2020

@kdawson -- I took a quick look at the code in LO2 and in Concord (the outliner that LO2 is a wrapper for) and they of course use the arrow keys, but don't appear to do anything with cmd-arrowkeys or cmd-opt-arrowkeys. I'm not sure how the browser distributes control in this situation, we're just using jQuery to capture the keystrokes.

So I'm not sure where to look from here. Are you a developer?

@kdawson

This comment has been minimized.

Copy link
Author

@kdawson kdawson commented Feb 3, 2020

Not a developer; a tyro perhaps. I have noticed that some websites capture cmd-opt-arrow but in LO2's case had the opportunity to report it. I wonder whether it's something obscure with jQuery, some mode it gets in or some background condition that makes it grab those keystrokes. In LO2's case, it seems odd that it throws away the modifiers and delivers plain arrow keys to the app.

@mauskin

This comment has been minimized.

Copy link

@mauskin mauskin commented Feb 3, 2020

The issue might be that the checks for keyspresses are too broad.

From a quick glance at concord.js I assume that the part that captures keys checks only for presses, but not if certain keys aren't pressed. For example Cmd+L will be captured in all cases when those two key are pressed, meaning on Cmd+L, Cmd+Shift+L, Cmd+Alt+L, etc. Unless other actions are defined.

@scripting

This comment has been minimized.

Copy link
Owner

@scripting scripting commented Feb 3, 2020

You can see the code in concord.js. If you have a suggestion of a different way to handle keystrokes, I'm ready to hear it. ;-)

@mauskin

This comment has been minimized.

Copy link

@mauskin mauskin commented Feb 4, 2020

Here’s a rough idea I had. Let’s call it a first draft so I don’t deel bad about the code clarity :)

Each meta key combination is a unique thing. To avoid checking for all the possible ones we set a convention for the names of combinations. Then on each key press we just set all the combos according to that convention. Let me show you an example from concord.js. This one should fix the issue that started this:

case 37:
	// left
	var active = false;
	if($(event.target).hasClass("concord-text")) {
		if(event.target.selectionStart > 0) {
			active = false;
			}
		}
	if(context.find(".concord-cursor.selected").length == 1) {
		active = true;
		}
	if(active && metaCombo.none) { // <- here you check that no meta keys were pressed
		keyCaptured = true;
		event.preventDefault();
		var cursor = concordInstance.op.getCursor();
		var prev = concordInstance.op._walk_up(cursor);
		if(prev) {
			concordInstance.op.setCursor(prev);
			}
		}
	break;

And another one:

case 68:
	//CMD+D
		if(metaCombo.cmd) { // <- check for exact key combination
			keyCaptured = true;
			event.preventDefault();
			concordInstance.op.reorg(down);
		}
		break;

I works by assigning the return value of the function below to a metaCombo variable at the start of event handler.

That way the check makes sure only the exact combination of meta keys is pressed. More complex ones would me metaCombo.cmd_ctrl_alt, metaCombo.ctrl_shift, etc.

Here’s the function that calculates the meta combos:

function getMetaCombo(event) {
  const combos = Object.create(null);
  
  // all meta keys we track and their aliases that you can set to in a familiar to you way 
  const metaKeys = {
    metaKey: 'cmd',
    ctrlKey: 'ctrl',
    altKey: 'alt',
    shiftKey: 'shift'
  };
  
  // set all the possible combinations of meta keys
  for (let i = 0; i < 16; i += 1) {
    if (i === 0) {
      combos.none = false;
      continue;
    }  
    const sig = i
      .toString(2)
      .padStart(4, '0')
      .split('')
      .reduce(function (name, on, index) {
        if (Number(on)) {
          name.push(metaKeys[Object.keys(metaKeys)[index]])
        }
        return name;
      }, [])
      .join('_');
    combos[sig] = false;
  }
  
  // set the current combination pressed to `true`
  const comboPressed = Object.keys(metaKeys).reduce(function (name, key) {
    if (event[key]) {
      name.push(metaKeys[key]);
    }
    return name;
  }, []).join('_');
  
  if (comboPressed === '') {
    combos.none = true;
  } else {
  	combos[comboPressed] = true;
  }
  
  return combos;
}

Sorry for not being breif or clear enough.

@clartaq

This comment has been minimized.

Copy link

@clartaq clartaq commented Feb 4, 2020

One way I've done this is to match against maps of the key and the modifiers. For example, in ClojureScript, I do this:

(defn handle-key-down-for-outline
  "Handle key-down events and dispatch them to the appropriate handlers."
  [aps root-ratom evt topic-ratom span-id]
  (let [key-map (key-evt->map evt)
        args {:aps         aps
              :root-ratom  ...
     ...
    (cond
      ...
      (= key-map {:key "Enter" :modifiers (merge-def-mods {:ctrl true})})
      (split-headline! args)

The handler receives a key event, evt, and converts it to a map containing the key and modifier elements of the event (using the key-evt->map function). Then, in the big long conditional, it compares that map to what you have defined to be the combination to execute some command.

The clause above detects the Ctrl-Enter key and modifier to initiate splitting a headline. (The merge-def-mods is just a convenience function so I don't have to write out the entire map every time.)

This has the advantage that you must match the state of each modifier as well as the named key before the command is executed. This is in Clojure/Script. I don't know if Javascript lets you compare whole maps like this.

@mauskin

This comment has been minimized.

Copy link

@mauskin mauskin commented Feb 5, 2020

@clartaq does merge-def-mods considers left out keys as to be false? Will that example run split-headline when Ctrl+Shift+Enter is pressed?

The issue seems to be that the code catches key shortcuts too broadly. Here what it does:

  1. Catch the key press by its code
  2. Check if desired modifier is pressed
  3. Run the code

What’s missing is the check if all the other modifier keys were left alone.

I also thought about merging the objects somehow, but I didn‘t like the “ergonomics” of it. On each check the developer would need to type out the object literal with modifiers as keys and values as the state.

What I wanted to achieve with my approach is to prefabricate all the combinations as metaCombos object keys in a conventional way. That way one could (a) remember how each of fifteen combinations is called, (b) set up their IDE to suggest those names (via JSDoc or something).

The convention is this order: Cmd, Ctrl, Alt, Shift.

@clartaq

This comment has been minimized.

Copy link

@clartaq clartaq commented Feb 5, 2020

@mauskin To answer your questions:

  1. Yes, the merge-def-mods function does include settings for modifier keys that should be left un-pressed. Here is how that is defined:
(defn- def-mods
  "Return a map containing the default values for keyboard modifiers."
  []
  {:ctrl false :alt false :shift false :cmd false})

(defn- merge-def-mods
  "Merge a map of modifiers (containing any modifiers which should be present)
  with a default map of false values for all modifiers."
  [m]
  (merge (def-mods) m))

The def-mods function just returns a map for containing key/value pairs (in the map sense) for the modifiers I am interested in. The merge-def-mods function merges that map with its argument, another map where the values for some of the modifiers might be true. (In the case of a key collision between the argument and def-mods, the merge function will use the value from the second map.) For the example above the output would be: {:ctrl true, :alt false, :shift false, :cmd false}, thus the entire map to be matched would be {:key Enter, :modifiers {:ctrl true, :alt false, :shift false, :cmd false}}.

  1. No, pressing Ctrl+Shift+Enter does not run the split-headline! function. The chord Ctrl+Shift+Enter just happens to be the keyboard shortcut that I use to run the join-headlines! function.

The comparison with the key-map, (= key-map..., must be exact, including modifier keys that must remain un-pressed.

I just use the merge-def-mods function so I don't have to type out the complete map for every keyboard shortcut of interest. I don't enforce any order in pressing the keys or in their order in the map.

I too hesitated before implementing this approach, but for fear of performance problems because all of the maps for key comparisons are generated on the fly, every time any key is pressed. In practice it is not an issue.

If it ever becomes an issue, I believe I can use a simple macro to turn those maps into compile-time constants, but that is more complexity than I want to deal with right now.

There are still shortcomings of this approach.

  • It doesn't consider if modifier keys were pressed on the right or left side of the keyboard.
  • There is no provision for handling macOS keyboards differently than Windows or Linux keyboards.

And that is ignoring the biggest issue, which in my experience, has been inconsistency between browsers, dealing with browser plugins that fiddle with key bindings, and not messing up accessibility conventions. But that is a story for another question (or blog rant).

@scripting

This comment has been minimized.

Copy link
Owner

@scripting scripting commented Feb 5, 2020

Just want to note that I am following this thread. Haven't been able to get to digging in, but I will.

@mauskin

This comment has been minimized.

Copy link

@mauskin mauskin commented Feb 6, 2020

There is no provision for handling macOS keyboards differently than Windows or Linux keyboards.

Yes that one is tricky. Myself switching back and forth between Mac and PC I can say there’s no safe way to handle it. Each shortcut needs to be decided on individually.

@scripting

This comment has been minimized.

Copy link
Owner

@scripting scripting commented Feb 6, 2020

First thanks to the group that has assembled here. There seems to be a lot of knowledge here.

There's one thing I'm not seeing, how, from a JavaScript app can I examine a keyboard event to find out which modifier keys are pressed? I've been puzzling through teh various bits of code posted here, and done a fair number of searches, and I don't see it.

That's the place I need to begin this trip.

Also, I have written a keystroke manager for Concord, have been using it for a long time, but have not yet released it publicly. I plan to.

I've uploaded the code to a gist.

@scripting

This comment has been minimized.

Copy link
Owner

@scripting scripting commented Feb 6, 2020

Never mind the above post. I did an experiment and realize the keyboard is more low level than I remembered. Have to do some thinking now.

@clartaq

This comment has been minimized.

Copy link

@clartaq clartaq commented Feb 6, 2020

I use the keydown event. I recall having flaky cross-browser problems with keyup and keypress (event, not the library). And I've been using the key property of the event, not the code, charCode, keyCode, or which properties for similar reasons. Might be better now though.

@scripting

This comment has been minimized.

Copy link
Owner

@scripting scripting commented Feb 6, 2020

OK, going all the way back to the top of the thread, to the feature request from the user, @kdawson -- I have made the change in the test version, and verified that it now no longer consumes cmd-arrowkeys. So your modifications should work. I want to do some more testing, and fix a few other things before releasing. Thanks for the initial report, I love making the software work better for more people. ;-)

@scripting scripting closed this Feb 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.