Skip to content

Unify gateway offline screens#36324

Merged
gzdunek merged 12 commits intomasterfrom
gzdunek/unify-gateway-offline-screen
Jan 10, 2024
Merged

Unify gateway offline screens#36324
gzdunek merged 12 commits intomasterfrom
gzdunek/unify-gateway-offline-screen

Conversation

@gzdunek
Copy link
Copy Markdown
Contributor

@gzdunek gzdunek commented Jan 5, 2024

Currently, we have different loading and error screens for db and kube gateways. For example, the kube gateway uses document progress bar, while the db gateway uses inline progress. Also, kube gateway doesn't show any text when it is setting up.
Soon we will have another gateway type, so I wanted to have a single component that we could use there. I took an inspiration from the updated DocumentCluster look, which I think is quite nice.

Before

Database gateway:
image

Kube gateway:
image

After

Database gateway:
image

Kube gateway:
image

This field is set in `DocumentGatewayKube.tsx`
when creating a gateway,
but it wasn't explicitly defined on the type (it works because of loose types of `documentsService.update`).
@gzdunek gzdunek added the no-changelog Indicates that a PR does not require a changelog entry label Jan 5, 2024
Comment thread web/packages/teleterm/src/ui/DocumentGatewayKube/DocumentGatewayKube.tsx Outdated
Comment thread web/packages/teleterm/src/ui/components/OfflineGateway.tsx Outdated
@gzdunek gzdunek requested a review from ravicious January 5, 2024 12:01
@gzdunek gzdunek marked this pull request as ready for review January 5, 2024 12:01
@gzdunek gzdunek requested review from avatus and removed request for ibeckermayer and kimlisa January 5, 2024 12:02
@gzdunek gzdunek force-pushed the gzdunek/unify-gateway-offline-screen branch from a682d46 to 576da3b Compare January 5, 2024 12:04
@gzdunek gzdunek mentioned this pull request Jan 8, 2024
1 task
@avatus
Copy link
Copy Markdown
Contributor

avatus commented Jan 8, 2024

I really like the bold name on its own above the rest of the text instead of inline. This looks great

Copy link
Copy Markdown
Member

@ravicious ravicious left a comment

Choose a reason for hiding this comment

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

Looks good! Though I think we shouldn't center the loading state.

Comment thread web/packages/teleterm/src/ui/DocumentGatewayKube/DocumentGatewayKube.tsx Outdated
The database connection is {statusDescription}
{/* TODO(ravicious): Use doc.status instead of LinearProgress. */}
{props.connectAttempt.status === 'processing' && <LinearProgress />}
<Flex flexDirection="column" m="auto" alignItems="center" maxWidth="500px">
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm not sure if we should center it. We don't really have any other documents that do this, do we? When the app is almost fullscreen, especially on bigger screens, the transition from a centered loading state to a success state that's aligned to the top is going to look a bit weird.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We have a very similar screen in DocumentCluster where we center the content.
But I think you are right, we should rather align it to the top. I changed it in both places.

Now it looks like below:
image

Comment thread web/packages/teleterm/src/ui/components/OfflineGateway.tsx Outdated
@gzdunek gzdunek added this pull request to the merge queue Jan 10, 2024
@gzdunek gzdunek removed this pull request from the merge queue due to a manual request Jan 10, 2024
@gzdunek gzdunek enabled auto-merge January 10, 2024 14:45
@gzdunek gzdunek added this pull request to the merge queue Jan 10, 2024
Merged via the queue into master with commit ae6c4e5 Jan 10, 2024
@gzdunek gzdunek deleted the gzdunek/unify-gateway-offline-screen branch January 10, 2024 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changelog Indicates that a PR does not require a changelog entry size/sm ui

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants