Skip to content

Apple iPad - Smart Keyboard Arrows - v3#1065

Merged
parisk merged 2 commits into
xtermjs:v3from
exsilium:iPadHWArrowsV3
Oct 31, 2017
Merged

Apple iPad - Smart Keyboard Arrows - v3#1065
parisk merged 2 commits into
xtermjs:v3from
exsilium:iPadHWArrowsV3

Conversation

@exsilium

Copy link
Copy Markdown
Contributor

A duplicate of #1064 against the v3 branch. Please test and provide feedback.

  • Arrows trigger keydown event
  • keyCode == 0, always.
  • The actual key pressed is identified via key strings:
    • UIKeyInputUpArrow
    • UIKeyInputLeftArrow
    • UIKeyInputRightArrow
    • UIKeyInputDownArrow

@parisk parisk self-assigned this Oct 16, 2017
@exsilium exsilium changed the title Smart Keyboard Arrows - v3 Apple iPad - Smart Keyboard Arrows - v3 Oct 16, 2017
@parisk

parisk commented Oct 16, 2017

Copy link
Copy Markdown
Contributor

@exsilium code looks OK, but I can't get this working on my iPad Pro, with the smart keyboard on.

Do you have any debugging steps that I have to take to see what might be going wrong?

@parisk

parisk commented Oct 16, 2017

Copy link
Copy Markdown
Contributor

btw, I did the following quick hack to see what might be going wrong and seems like it never gets into the keyCode == 0 case:

    switch (ev.keyCode) {
      case 0:
        alert('Zero keyCode!')
        alert(ev.key);
        if (ev.key === 'UIKeyInputUpArrow') {
          if (this.applicationCursor) {
            result.key = C0.ESC + 'OA';
          } else {
            result.key = C0.ESC + '[A';
          }
        }

@exsilium

exsilium commented Oct 16, 2017

Copy link
Copy Markdown
Contributor Author

Sad to hear.. do you get any keydown event triggered when using the arrows? e.g. pull those alert()'s before the keyCode check happens? Also, what iOS version are you using? My device is on 11.0.3

@parisk

parisk commented Oct 16, 2017

Copy link
Copy Markdown
Contributor

I am on iOS 11.0.3 as well, using Safari (iPad Pro 10.9).

Seems like the arrows do not fire any of the keydown, keypress, keyup events, but they work fine on both ProseMirror (http://prosemirror.net/) and CodeMirror (http://codemirror.net/).

Both of these libraries though do not seem to handle the above events (e.g. UIKeyInputUpArrow) anywhere in their codebases so, is there any chance we have to handle this with a different event type?

@exsilium

Copy link
Copy Markdown
Contributor Author

I'm on 12.9 pro (2017) also on Safari, so it's unclear why I'm getting those events triggered - is it possible, that the bigger Smart Keyboard works differently? 😕 . The text editor examples rely on composition change to detect the arrow keys. Here's how ace does it.

@exsilium

Copy link
Copy Markdown
Contributor Author

@parisk - a simple test, does your iPad keyboard give any keycode entry when you press an arrow key at this site: http://keycode.info

@exsilium

exsilium commented Oct 16, 2017

Copy link
Copy Markdown
Contributor Author

This seems to be specific to the demo app, I'm unable to get those events triggered while using the xterm.js bundled demo. However, it works when I'm using the xterm within https://github.com/exsilium/cloud9/tree/development (feel free to try this yourself, you do need to change the package.json to use the patch #1064 version of xterm.js)

@parisk

parisk commented Oct 17, 2017

Copy link
Copy Markdown
Contributor

OK, I think I found the root cause of this.

For some reason it seems that the smart keyboard events are being fired only when listening to events on document.body.

Can you give it a try here: http://qizvas6t.apps.lair.io?

If this is the case, we should reconsider the way we are listening to events (at least on mobile devices).

@Tyriar

Tyriar commented Oct 17, 2017

Copy link
Copy Markdown
Member

Will this issue go away when we move to listening to the input event? We need to do this anyway to cover other IMEs such as the emoji picker on Windows and Mac.

@exsilium

Copy link
Copy Markdown
Contributor Author

@parisk I might have been late to the party, the lair link states that the application "has an error and cannot start" :(

@parisk

parisk commented Oct 18, 2017

Copy link
Copy Markdown
Contributor

@exsilium sorry about that, we shutdown projects after a few hours of inactivity 😅.

@Tyriar not sure. Only if this works out of the box, because the input event does not seem to provide any information about the key pressed.

We could use a mix of all different events and we could try listening on document.body and making sure that the event target/source is the terminal before we act.

@exsilium

Copy link
Copy Markdown
Contributor Author

@parisk ok, let me know if it ever comes back. i'll try to be quicker about it to test it :) Cheers! 🍻

@parisk

parisk commented Oct 18, 2017

Copy link
Copy Markdown
Contributor

@exsilium if you are in the mood of testing, please drop me a ping at https://gitter.im/sourcelair/xterm.js to put it online. Maybe we will be able to hash out a solution in a few minutes also 😄 .

@parisk

parisk commented Oct 23, 2017

Copy link
Copy Markdown
Contributor

@exsilium I propose the following:

  1. We detect the platform via src/utils/Browser.ts
  2. If the platform is iPad, we listen to the keydown event on document
  3. If keyCode is equal to 0 we forward the event to _keyDown

How does that sound?

@exsilium

Copy link
Copy Markdown
Contributor Author

Hi @parisk ! This sounds good 👍 Will you be making the needed changes and get this merged?

@parisk

parisk commented Oct 23, 2017

Copy link
Copy Markdown
Contributor

@exsilium sure.

@parisk parisk mentioned this pull request Oct 31, 2017
@parisk

parisk commented Oct 31, 2017

Copy link
Copy Markdown
Contributor

Well, my plan didn't work at all 😅. I spent the last days trying to debug this but had no luck with it.

Let's merge this as-is to solve at least some cases and have a guide for the future.

I also created an issue to follow-up on iOS keyboard support: #1101

@parisk parisk merged commit 36a155d into xtermjs:v3 Oct 31, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants