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

[v9] Backport Teleport Connect (2 of 5) #763

Merged
merged 26 commits into from
Apr 27, 2022

Conversation

@ravicious ravicious marked this pull request as ready for review April 26, 2022 11:12
@ravicious ravicious requested a review from kimlisa April 26, 2022 11:16
@@ -75,6 +73,12 @@ export default function useGateway(doc: types.DocumentGateway) {
}
}, [disconnectAttempt.status]);

useEffect(() => {
if (cluster.connected) {
Copy link
Contributor

Choose a reason for hiding this comment

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

is optional chaining required here? just saw it on line 49

Copy link
Member Author

Choose a reason for hiding this comment

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

Now that I look at it, it seems that the checks for cluster being null are unnecessary here. 🤔 If someone logs out from the cluster (and thus removes it) we remove any open resources from that cluster as well. We'll look into removing that after the preview release, thanks!

Comment on lines +26 to +28
css={`
white-space: nowrap;
`}
Copy link
Contributor

Choose a reason for hiding this comment

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

just curious between css and style, do both achieve the same thing (just syntax is a bit different?)

Copy link
Member Author

Choose a reason for hiding this comment

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

@gzdunek Can you answer Lisa's question here? I remember you were changing that. ;)

Copy link
Contributor

Choose a reason for hiding this comment

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

css works the same as styled(Text)... so you have access to the theme for example. Also, css generates a css class, while style adds inline style to the element.

Copy link
Contributor

Choose a reason for hiding this comment

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

@gzdunek i see, thanks! should we be using css from now on? or does it matter?

Copy link
Contributor

Choose a reason for hiding this comment

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

@kimlisa I think we can prefer css over style as it has more capabilities (and nicer syntax) :)

@ravicious ravicious changed the base branch from ravicious/v9/backport-connect-1-of-5 to teleport-v9 April 27, 2022 09:11
@ravicious ravicious changed the base branch from teleport-v9 to ravicious/v9/backport-connect-1-of-5 April 27, 2022 09:14
gzdunek and others added 24 commits April 27, 2022 11:38
This commit removes a bunch of unused protobufs and also updates some of
those that got updated on the teleterm branch in the teleport repo.

The biggest change from all of them is that LoginRequest now has oneof
on Sso and Local. [1] This is because a login request should have either
Sso or Local params, but not both at the same time.

The previous implementation called both `setSso` and `setLogin` on the same
object. This no longer works with the use of `oneof`, because calling `setLogin`
after `setSso` would clear the `Sso` params. [2]

[1] gravitational/teleport#10286 (comment)
[2] https://developers.google.com/protocol-buffers/docs/proto3#oneof_features
@ravicious ravicious force-pushed the ravicious/v9/backport-connect-2-of-5 branch from ebb197b to 5ded89c Compare April 27, 2022 09:41
@ravicious ravicious changed the base branch from ravicious/v9/backport-connect-1-of-5 to teleport-v9 April 27, 2022 09:41
@ravicious ravicious merged commit a550a63 into teleport-v9 Apr 27, 2022
@ravicious ravicious deleted the ravicious/v9/backport-connect-2-of-5 branch April 27, 2022 09:47
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.

4 participants