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

Add ability to select files by typing a prefix #2190

Merged
merged 2 commits into from May 11, 2017

Conversation

@psuter
Copy link
Contributor

@psuter psuter commented May 11, 2017

A proposed solution to #1059.

In action: https://vid.me/jMAQ (note that the overlay displaying the keys is from the OS, not the JupyterLab UI).

Copy link
Member

@blink1073 blink1073 left a comment

Thanks for this!

// Detects printable characters typed by the user.
// Not all browsers support .key, but it discharges us from reconstructing
// characters from key codes.
if (event.key !== undefined && event.key.length === 1) {
Copy link
Member

@blink1073 blink1073 May 11, 2017

Choose a reason for hiding this comment

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

We can import getKeyboardLayout from @phosphor/keyboard and use .keyForKeydownEvent to get the correct value. https://github.com/phosphorjs/phosphor/blob/297d7d247d69de6d329336ab6e30fd26bd4c204f/packages/keyboard/src/index.ts#L50

Copy link
Contributor Author

@psuter psuter May 11, 2017

Choose a reason for hiding this comment

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

I'm not sure this would properly detect, e.g. _? from https://github.com/phosphorjs/phosphor/blob/297d7d247d69de6d329336ab6e30fd26bd4c204f/packages/keyboard/src/index.ts#L143 it looks like it's only switching on .keyCode, while it would also need to look at which modifiers are pressed (since _ is Shift+-).

Copy link
Member

@blink1073 blink1073 May 11, 2017

Choose a reason for hiding this comment

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

Ah, you are correct. We had been using this for keyboard shortcuts, where the modifier is specified separately.

@ellisonbg
Copy link
Contributor

@ellisonbg ellisonbg commented May 11, 2017

Played around with it interactively:

  • A little difficult to tell when the file browser is focused, but that is separate from this PR.
  • Would be nice to have a keyboard shortcut to open an item once the keyboard is on it.

@psuter
Copy link
Contributor Author

@psuter psuter commented May 11, 2017

Would be nice to have a keyboard shortcut to open an item once the keyboard is on it.

I noticed that Enter is mapped to "Rename" for Mac users (https://github.com/jupyterlab/jupyterlab/blob/master/packages/filebrowser/src/listing.ts#L825). I imagine this is to mimic the behavior of Finder. I also found myself naturally pressing Enter after a match (and disabling that check did the trick).

@blink1073
Copy link
Member

@blink1073 blink1073 commented May 11, 2017

Happy to remove the Enter behavior given this new capability.

@ellisonbg
Copy link
Contributor

@ellisonbg ellisonbg commented May 11, 2017

Yes, let's use Enter for open!

@ellisonbg
Copy link
Contributor

@ellisonbg ellisonbg commented May 11, 2017

@blink1073 done with review? I think this is ready...

@blink1073
Copy link
Member

@blink1073 blink1073 commented May 11, 2017

Going to test locally before merging.

@blink1073
Copy link
Member

@blink1073 blink1073 commented May 11, 2017

This is 🔥, thanks @psuter!

@blink1073 blink1073 merged commit 17c2e00 into jupyterlab:master May 11, 2017
2 checks passed
@psuter psuter deleted the files-prefix-select branch May 11, 2017
@blink1073 blink1073 mentioned this pull request May 16, 2017
@ian-r-rose ian-r-rose mentioned this pull request May 19, 2017
@lock lock bot locked as resolved and limited conversation to collaborators Aug 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants