Skip to content
This repository has been archived by the owner on Jan 19, 2023. It is now read-only.

Initial implementation of quick-switcher (keyboard-driven navigation) #434

Merged
merged 3 commits into from
Dec 12, 2019

Conversation

alexbrand
Copy link
Contributor

Initial implementation of a quick-switcher / keyboard-driven navigation.

I created a new component that is attached to the root of the application. The component leverages the Clarity modal functionality.

To open the quick-switcher, hit the k key. Once open, you can use the input field to filter the list of potential destinations. You can use the arrow keys to go up and down in the list. Once you are set on a destination, hit Enter to navigate there. You may also use the mouse to select an item from the list.

Demo:
2019-11-19 11 15 56

Fixes #413

@alexbrand
Copy link
Contributor Author

Looks like the frontend build is failing. Going to review the build logs and address any issues.

@alexbrand alexbrand force-pushed the quick-switcher branch 2 times, most recently from b90f245 to 24d70ce Compare November 19, 2019 17:30
@alexbrand
Copy link
Contributor Author

Fixed the lint errors, minus one:

ERROR: /go/src/github.com/vmware-tanzu/octant/web/src/app/components/quick-switcher/quick-switcher.component.ts[90, 18]: non-arrow functions are forbidden

I need help with focusing the input once the Modal pops up. I am not sure how to do that in an Angular-native manner. Link to code in PR: https://github.com/vmware-tanzu/octant/pull/434/files#diff-cdf196737b682f9089d6e6506bfcebb2R88-R92

@wwitzel3
Copy link
Contributor

Oh, it just wants you to use () => {} instead of function () {}

@alexbrand
Copy link
Contributor Author

Aaaah okay! I am still wondering if there is a better way to focus on the input. Ideally, I should be able to get an event or something that indicates the modal has opened. I couldn’t find how to do that though.

@bryanl
Copy link
Contributor

bryanl commented Nov 22, 2019

I like where this going. Is k the right invoking key? I'd like to throw some other contenders in the ring:

O for object
/ Because that's what vim does

Some other things to think about:

  • how would users discover this feature?
  • Could we reuse this modal idiom for other features? Perhaps a way to kickstart a process for creating objects?

@alexbrand
Copy link
Contributor Author

I don't have strong opinions on k being the invoking key. I initially went with it because that's what I use for kubectl on the command line.

I like the alternatives you propose @bryanl. Here is another one:

  • g for "Go" (github uses this: g + n goes to notifications for example).

I love the idea of using /, but I would use it to search the contents of a page.

The way I envision this:

  • k + pods + enter takes me to the pods page
  • / focuses on a search / filter bar
  • nginx to filter pods by name = nginx

re: feature discovery - I am not sure. I just reviewed Slack, Gmail and GitHub and none seem to have anything in the UI for discovering keyboard shortcuts. One thing they do have in common is that they all use ? or cmd + ? to bring up a help menu that explains the keyboard shortcuts thought.

@alexbrand
Copy link
Contributor Author

Hey all, anything I can do to move this forward?

Signed-off-by: Alexander Brand <alexbrand09@gmail.com>

// TODO(abrand): Hack for focusing on input. Need to figure this out.
const el = this.el;
setTimeout(function() {
Copy link
Contributor

@GuessWhoSamFoo GuessWhoSamFoo Dec 9, 2019

Choose a reason for hiding this comment

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

TSLint wants this to be setTimout(() => {

EDIT: Also looking into passing an event here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it would be great to use some kind of event instead of relying on setTimeout. I just couldn't figure it out. Would love some help here.

@GuessWhoSamFoo
Copy link
Contributor

@alexbrand Also can you add QuickSwitcherComponent to the declarations in app.component.spec.ts?

Signed-off-by: Alexander Brand <alexbrand09@gmail.com>
@alexbrand
Copy link
Contributor Author

All set @GuessWhoSamFoo 👍

@GuessWhoSamFoo
Copy link
Contributor

GuessWhoSamFoo commented Dec 11, 2019

Two small changes then lgtm

  1. For e2e testing, https://github.com/vmware-tanzu/octant/blob/master/web/cypress/integration/deployment.spec.js#L22 should be replaced with cy.contains('span[class="ng-value-label ng-star-inserted"]', result.stdout); because there are now two focus elements possible (one on the namespace dropdown and the other from quick switcher)

  2. Linter comma

ERROR: /go/src/github.com/vmware-tanzu/octant/web/src/app/app.component.spec.ts[52, 31]: Insert `,`

Signed-off-by: Alexander Brand <alexbrand09@gmail.com>
@GuessWhoSamFoo GuessWhoSamFoo changed the title WIP: Initial implementation of quick-switcher (keyboard-driven navigation) Initial implementation of quick-switcher (keyboard-driven navigation) Dec 12, 2019
@GuessWhoSamFoo GuessWhoSamFoo merged commit 9f07149 into vmware-archive:master Dec 12, 2019
@GuessWhoSamFoo
Copy link
Contributor

Thanks again @alexbrand !

@alexbrand alexbrand deleted the quick-switcher branch December 13, 2019 14:33
@alexbrand alexbrand restored the quick-switcher branch December 13, 2019 14:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add keyboard shortcuts to navigate UI
4 participants