add {cmd,ctrl} + l for wunderbar focus #2003

2018-10-04T07:56:49+02:00

neb-b commented

2018-10-04 07:56:49 +02:00

(Migrated from github.com)

For part of #1395

tested on windows
tested on linux
tested on mac

Also got rid of prop types and unused files

For part of #1395 - [x] tested on windows - [x] tested on linux - [x] tested on mac Also got rid of prop types and unused files

skhameneh (Migrated from github.com) reviewed 2018-10-05 07:47:35 +02:00

src/renderer/component/wunderbar/internal/autocomplete.jsx

skhameneh (Migrated from github.com) commented

2018-10-05 07:45:36 +02:00

Why is this commented out?

src/renderer/component/wunderbar/view.jsx

					
				@ -40,1 +52,4 @@

				  handleKeyDown(event: SyntheticKeyboardEvent<*>) {

				    const { ctrlKey, metaKey, keyCode } = event;

				    const { doFocus, doBlur, focused } = this.props;

skhameneh (Migrated from github.com) commented

2018-10-05 07:47:10 +02:00

Does this correctly remove the event? Using bind above creates a handle that isn't stored.

Does this correctly remove the event? Using `bind` above creates a handle that isn't stored.

neb-b (Migrated from github.com) reviewed 2018-10-05 20:05:19 +02:00

src/renderer/component/wunderbar/internal/autocomplete.jsx

neb-b (Migrated from github.com) commented

2018-10-05 20:05:19 +02:00

I wanted to get rid of prop-types since we aren't using it. This is from a third party library but we needed to make a change to the component so I just copied it in.

I still wanted to keep them for documentation.

I wanted to get rid of `prop-types` since we aren't using it. This is from a third party library but we needed to make a change to the component so I just copied it in. I still wanted to keep them for documentation.

neb-b (Migrated from github.com) reviewed 2018-10-05 20:05:59 +02:00

src/renderer/component/wunderbar/view.jsx

					
				@ -40,1 +52,4 @@

				  handleKeyDown(event: SyntheticKeyboardEvent<*>) {

				    const { ctrlKey, metaKey, keyCode } = event;

				    const { doFocus, doBlur, focused } = this.props;

neb-b (Migrated from github.com) commented

2018-10-05 20:05:59 +02:00

👍 I'll move the bind into the constructor and use this.handleKeyDown for both.

👍 I'll move the `bind` into the constructor and use `this.handleKeyDown` for both.

kauffj commented

2018-10-05 20:16:12 +02:00

(Migrated from github.com)

Love the addition of keyboard shortcuts.

This isn't a PR blocker and can be considered more for the next round/addition, but if keyboard shortcuts are intended to be global, a design that allows them to be cleanly separated and grouped is likely preferable to one that puts them throughout individual components.

Love the addition of keyboard shortcuts. This isn't a PR blocker and can be considered more for the next round/addition, but if keyboard shortcuts are intended to be global, a design that allows them to be cleanly separated and grouped is likely preferable to one that puts them throughout individual components.

👍 1

skhameneh (Migrated from github.com) approved these changes 2018-10-09 19:03:31 +02:00

Sign in to join this conversation.

No reviewers

skhameneh