Skip to content
This repository has been archived by the owner on Feb 8, 2024. It is now read-only.

Improve identity picker #670

Merged
merged 5 commits into from
Mar 16, 2022
Merged

Improve identity picker #670

merged 5 commits into from
Mar 16, 2022

Conversation

gzdunek
Copy link
Contributor

@gzdunek gzdunek commented Mar 14, 2022

  • layout/styling improvements
  • added Logout
  • added Add Cluster
  • added Remove Cluster

@gzdunek gzdunek requested a review from ravicious March 14, 2022 14:44
onClose={() => setIsLongInfoOpened(false)}
open={isPopoverOpened}
anchorEl={selectorRef.current}
anchorOrigin={{ vertical: 'bottom', horizontal: 'right' }}
Copy link
Member

Choose a reason for hiding this comment

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

This fixes the error that was popping up in the console, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes

ctx.clustersService.useState();
ctx.workspacesService.useState();

function changeContext(clusterUri: string): Promise<void> {
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps this could be changeRootCluster, changeWorkspace or refer to some other established concept? I'm not 100% sure but I don't think we use context to mean "root cluster"/"workspace" anywhere else in the code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct, changed to changeRootCluster.

Comment on lines +108 to +109
// clusterUri can be undefined - we don't want to create a workspace for it
if (clusterUri && !draftState.workspaces[clusterUri]) {
Copy link
Member

Choose a reason for hiding this comment

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

Does an undefined clusterUri mean some kind of empty state when the user launches the app for the first time? I suppose for now this is good enough way to handle this case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes - it is an empty state. clusterUri is empty when you don't have an active cluster or it is the first launch. In that case we will show Connect to cluster screen (next PR) so this case will be handled.

Comment on lines +63 to +65
// Hack - for some reason xterm.js doesn't allow moving a focus to the Identity popover
// when it is focused using element.focus(). Moreover, it looks like this solution has a benefit
// of returning the focus to the previously focused element when popover is closed.
Copy link
Member

Choose a reason for hiding this comment

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

It's been a while since I ran into that, but I noticed that sometimes my own shortcuts defined through BetterTouchTool don't work while some field in Teleterm is focused.

Just putting it out there in case we run into similar problems in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's interesting. Does it happen in input field or in xterm area?

Copy link
Member

Choose a reason for hiding this comment

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

Not sure where it was, it was either the old command bar or xterm.

@gzdunek gzdunek merged commit ca132df into master Mar 16, 2022
@gzdunek gzdunek deleted the gzdunek/improve-identity-picker-2 branch March 16, 2022 11:29
ravicious pushed a commit that referenced this pull request Apr 26, 2022
ravicious added a commit that referenced this pull request Apr 27, 2022
* Add more tests to Teleterm (#601)

* add more tests to Teleterm

* Update command for generating gRPC files for Teleterm

* Add cluster context switching (#624)

* Add connections switcher (#647)

* Add keyboard support to `Connections` popover (#651)

* Add clusters picker (#668)

* Improve identity picker (#670)

* Reformat commandLauncher.ts

* Remove command palette commands from command launcher

The way their `run` function works conflicts with the new implementation
(it directly changes quickInputService's state), so we remove that in
this commit to have less to deal with in upcoming commits.

* Show autocomplete suggestions in command bar

This commit does not support actually choosing any suggestion yet, just
showing them in the UI.

* Remove code related to empty command bar item

Our new code isn't going to show anything when there are no matches.

* Remove old pickers, rename Item to Suggestion

* Autocomplete commands and ssh logins

* Ignore case for autocomplete

* Automatically append @ after ssh login suggestion

* QuickInputService.getAutocomplete: Return no-match on empty suggestions

This required adding all those mocks to the tests so that the returned
suggestions in some of the tests are not empty.

* Autocomplete ssh hostnames

* Open command bar commands in new local shell

* Append space after picking command suggestion

* Adjust how showing & hiding autocomplete works in command bar

1. Pressing Esc while suggestions are shown should first close the suggestions
and the second press should actually exit the input.

2. Picking a suggestion should close the autocomplete if we didn't append
anything to the token (see comment for details).

3. Typing something in the command bar should always show the autocomplete
in case there are suggestions to show.

Also remove unused call to setInputValue.

* Fix opening new terminal when there's no active document

* useQuickInput: Rename serviceQuickInput to quickInputService

* Include command to run in AutocompleteResult

* Create DocumentTshNode after executing "tsh ssh" in command bar

* Update teleterm styles (#674)

* Remove leftover cruft from quick pickers

* Launch unsupported invocations of tsh ssh in local shell

* add simple empty state to pickers

* adjust `QuickInput` to match designs

* always show active item in `QuickInputList`

* make middle part of `TopBar` central

Co-authored-by: Grzegorz Zdunek <[email protected]>
Co-authored-by: Grzegorz Zdunek <[email protected]>
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.

2 participants