Add improvements for the EKS Discover UI. #38609
Conversation
AntonAM
added
the
no-changelog
| result.error && | ||
| !result.error.message.includes( | ||
| 'teleport-kube-agent is already installed' | ||
| ) |
There was a problem hiding this comment.
does this have any specific error code we could check for instead?
There was a problem hiding this comment.
Checking for error code as Michael suggested is ideal, but if we're going to check for a substring in an error message, we should make sure to leave a note where the error message in defined in the backend saying that it needs to be kept in sync with this. This way if someone changes the error message, this won't break.
There was a problem hiding this comment.
Not really, it's not an AWS error, just our internal message.
There was a problem hiding this comment.
I've added a comment 👍
| @@ -293,6 +298,17 @@ export function EnrollEksCluster(props: AgentStepProps) { | |||
| {fetchClustersAttempt.status === 'failed' && !hasIamPermError && ( | |||
| <Danger mt={3}>{fetchClustersAttempt.statusText}</Danger> | |||
| )} | |||
| <Text mt={4} mb={-3}> | |||
There was a problem hiding this comment.
What is the purpose of the negative value here? are we doing it to try and not affect the positioning of something else?
There was a problem hiding this comment.
I copied it from RDS discover where we use similar styled note, I think it's just to make margin to look better
Negative margin:
vs 0 margin
Not really sure which one looks better, I can remove negative margin if you think it's not needed.
There was a problem hiding this comment.
its a nit at this point so feel free to ignore, but it'd probably make more sense to reduce the mt of the element right below it (the Select the AWS Region... Text element) than to use negative margin here.
There was a problem hiding this comment.
Makes sense, changed the margins 👍
| result.error && | ||
| !result.error.message.includes( | ||
| 'teleport-kube-agent is already installed' | ||
| ) |
There was a problem hiding this comment.
Checking for error code as Michael suggested is ideal, but if we're going to check for a substring in an error message, we should make sure to leave a note where the error message in defined in the backend saying that it needs to be kept in sync with this. This way if someone changes the error message, this won't break.
web/packages/teleport/src/Discover/Kubernetes/EnrollEKSCluster/EnrollEksCluster.tsx
Outdated
Show resolved
Hide resolved
AntonAM
force-pushed
the
anton/eks-ui-improvements
branch
from
e0ff679 to
093e8e0
Compare
Co-authored-by: Yassine Bounekhla <[email protected]>
public-teleport-github-review-bot
bot
removed the request for review
from kimlisa
This PR adds a few small improvements to the Discover EKS flow -