Skip to content

Simplify auto updates multi-cluster view#57709

Merged
gzdunek merged 3 commits intomasterfrom
gzdunek/simplif-management-view
Aug 19, 2025
Merged

Simplify auto updates multi-cluster view#57709
gzdunek merged 3 commits intomasterfrom
gzdunek/simplif-management-view

Conversation

@gzdunek
Copy link
Copy Markdown
Contributor

@gzdunek gzdunek commented Aug 8, 2025 •
edited
Loading

The last part of #50948. RFD.

After using the updater in the real app, I realized that it doesn't look great when there are multiple clusters.
The management view occupies too much space and its options are not clear enough.

Summary of changes:

  • All options are now radio options.
  • Use highest compatible version option is no longer disabled if there's no compatible version.
    • If you have clusters on incompatible versions, we show a warning that you have to choose one to continue receiving updates. But previously it was an irreversible decision, once you selected a cluster, you couldn't go back to the previous state. It was quite weird UX.
  • Improved helper text under each option.
    • For example, previously we used vague language like 19.0.0 client instead of just Teleport Connect 19.0.0.
  • The number of alerts has been minimized - we display errors and actions inline if there is a cluster list.

Updated view (I think it now looks closer to what you suggested during our call, Rafał :p)

image

@gzdunek gzdunek added no-changelog Indicates that a PR does not require a changelog entry backport/branch/v16 backport/branch/v17 backport/branch/v18 labels Aug 8, 2025
@github-actions github-actions bot requested review from kimlisa and rudream August 8, 2025 15:47
@gzdunek gzdunek requested review from ravicious and ryanclark and removed request for kimlisa and rudream August 8, 2025 15:48
ryanclark
ryanclark reviewed Aug 12, 2025
View reviewed changes
Comment thread web/packages/teleterm/src/ui/AppUpdater/AutoUpdatesManagement.tsx Outdated
disabled,
label: getClusterName(c.clusterUri),
helperText: `${c.toolsVersion} client, ${compatibility}.`,
label: `Use version from cluster ${getClusterName(c.clusterUri)}`,
Copy link
Copy Markdown
Member

@ryanclark ryanclark Aug 12, 2025

Choose a reason for hiding this comment

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

Should we put the cluster name in bold? (Maybe the version numbers too? Makes them stand out a bit vs the text)

Copy link
Copy Markdown
Member

@ravicious ravicious Aug 13, 2025

Choose a reason for hiding this comment

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

We could maybe even use just the cluster name as the label.

Copy link
Copy Markdown
Contributor Author

@gzdunek gzdunek Aug 18, 2025

Choose a reason for hiding this comment

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

Hmm yeah, we can just show the cluster name.

Maybe the version numbers too?

It seems to me that shortening the labels improves readability enough that it's not necessary.

ravicious
ravicious reviewed Aug 13, 2025
View reviewed changes
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.

This is definitely an upgrade!

Comment thread web/packages/teleterm/src/ui/AppUpdater/AutoUpdatesManagement.tsx Outdated
Comment thread web/packages/teleterm/src/ui/AppUpdater/AutoUpdatesManagement.tsx Outdated
Comment thread web/packages/teleterm/src/ui/AppUpdater/AutoUpdatesManagement.tsx Outdated
disabled,
label: getClusterName(c.clusterUri),
helperText: `${c.toolsVersion} client, ${compatibility}.`,
label: `Use version from cluster ${getClusterName(c.clusterUri)}`,
Copy link
Copy Markdown
Member

@ravicious ravicious Aug 13, 2025

Choose a reason for hiding this comment

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

We could maybe even use just the cluster name as the label.

Comment thread web/packages/teleterm/src/ui/AppUpdater/AutoUpdatesManagement.tsx Outdated
>
<Stack gap={0}>
<H3>Updates source</H3>
<P3>Multiple clusters can manage Teleport Connect version.</P3>
Copy link
Copy Markdown
Member

@ravicious ravicious Aug 13, 2025

Choose a reason for hiding this comment

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

What does "Multiple clusters can manage Teleport Connect version." really mean? It kind of suggests that you can pick multiple clusters but you can actually choose just one. Should this say something like "Choose which cluster to follow for updates."?

Copy link
Copy Markdown
Contributor Author

@gzdunek gzdunek Aug 18, 2025

Choose a reason for hiding this comment

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

I wanted a safe label that works in both cases: when clusters are incompatible and you need to select a cluster, and when clusters are compatible and no changes are needed (but you may want to pick a different cluster).

But actually "Choose which cluster to follow for updates." works fine too and doesn't suggest that you can select more than one option.

@gzdunek gzdunek requested review from ravicious and ryanclark August 18, 2025 09:06
@gzdunek gzdunek added this pull request to the merge queue Aug 19, 2025
Merged via the queue into master with commit b574045 Aug 19, 2025
40 checks passed
@gzdunek gzdunek deleted the gzdunek/simplif-management-view branch August 19, 2025 10:01
@backport-bot-workflows
Copy link
Copy Markdown
Contributor

backport-bot-workflows bot commented Aug 19, 2025

@gzdunek See the table below for backport results.

Branch Result
branch/v16 Failed
branch/v17 Failed
branch/v18 Failed

ryanclark pushed a commit that referenced this pull request Aug 19, 2025
@gzdunek @ryanclark
Simplify auto updates multi-cluster view (#57709)
be8df58 
* Simplify auto updates multi-cluster view

* Improve labels

* Fix tests
ryanclark pushed a commit that referenced this pull request Aug 19, 2025
@gzdunek @ryanclark
Simplify auto updates multi-cluster view (#57709)
feb7096 
* Simplify auto updates multi-cluster view

* Improve labels

* Fix tests
gzdunek added a commit that referenced this pull request Aug 22, 2025
@gzdunek
Simplify auto updates multi-cluster view (#57709)
00fd5b6 
* Simplify auto updates multi-cluster view

* Improve labels

* Fix tests

(cherry picked from commit b574045)
gzdunek added a commit that referenced this pull request Aug 22, 2025
@gzdunek
Simplify auto updates multi-cluster view (#57709)
0b034cd 
* Simplify auto updates multi-cluster view

* Improve labels

* Fix tests

(cherry picked from commit b574045)
This was referenced Aug 22, 2025
Merged
Merged
github-merge-queue bot pushed a commit that referenced this pull request Sep 2, 2025
@gzdunek @ravicious
[v17] Show app updater in UI (#58261)
d8cd3b4 
* Integrate app updater with UI (#57491)

* Implement `checkForUpdates`, `download`, `cancelDownload`, `changeManagingCluster` and `quitAndInstall`

* Expose new APIs through Electron IPC

* Add app updater context to store updater state

* Add `AppUpdates` dialog

* Switch "Download" button to "Starting Download..." immediately after clicking it

* Clear managing cluster on logout

* Return correct error in `GetClusterVersions`

* Do not prepend `https://` to file URL, DefaultBaseURL already contains it, trim the checksum value before validating it

* Only try to download an update if it's available

* Remove separate `will-quit` handler

* Clarify comment

* Invert condition in `preventInstallingOutdatedUpdates`

* Add comments

* Move `closeAllConnections()` out of try catch

* Add specialized functions to serialize/deserialize errors

* Add tests

* Leave TODO

* Do not wait for `maybeRemoveAppUpdatesManagingCluster` in `useClusterLogout`

* Hide app updater for .tar.gz which is not supported

* Add missing mock

(cherry picked from commit 13aa750)

* Show update widget in login dialog (#57695)

* Add app updates widget to login dialog

* Adjust compatibility warning to app updates

* Simplify widget error state, do not show full error message

* Make the widget take up 100% width both in login modal an in story

* Fix LocalWithPasswordless story

* Use `alignItems="stretch"`

* Use `SpaceProps` instead of redefining `mx` manually

* Infer type

* Display buttons in the bottom of Alert

* Avoid moving content when scrollbar appears

* Destructure `Promise.all` result

* Add comment

---------

Co-authored-by: Rafał Cieślak <rafal.cieslak@goteleport.com>
(cherry picked from commit c59a4e6)

* Simplify auto updates multi-cluster view (#57709)

* Simplify auto updates multi-cluster view

* Improve labels

* Fix tests

(cherry picked from commit b574045)

* Connect: Force focus after a successful SSO login

* Move login functions from ClustersService to useClusterLogin

* Remove no longer used GetResourcesParams

* Remove clusters/types.ts

* Force focus on Connect after successful SSO login

* Update SSO prompt after login but before sync

* Add test for SSO prompts

* Use `Promise.withResolvers`

Co-authored-by: Grzegorz Zdunek <gzdunek@users.noreply.github.com>

* Update wait-for-sync prompt message

---------

Co-authored-by: Grzegorz Zdunek <gzdunek@users.noreply.github.com>

* Hardcode the lowest supported Connect versions that support managed updates (#58348)

* Provide the lowest supported app versions for managed updates

* Deserialize errors in the UI code, not in preload

Otherwise, the error name is lost.

* Bump version in tests

* Lint

* Show exact version in error message

* Do not hide UnsupportedVersionError

* Fix margin

* Make alert take 100% width

(cherry picked from commit d7e1352)

---------

Co-authored-by: Rafał Cieślak <rafal.cieslak@goteleport.com>
github-merge-queue bot pushed a commit that referenced this pull request Sep 2, 2025
@gzdunek @ravicious
[v18] Show app updater in UI (#58260)
28315b3 
* Integrate app updater with UI (#57491)

* Implement `checkForUpdates`, `download`, `cancelDownload`, `changeManagingCluster` and `quitAndInstall`

* Expose new APIs through Electron IPC

* Add app updater context to store updater state

* Add `AppUpdates` dialog

* Switch "Download" button to "Starting Download..." immediately after clicking it

* Clear managing cluster on logout

* Return correct error in `GetClusterVersions`

* Do not prepend `https://` to file URL, DefaultBaseURL already contains it, trim the checksum value before validating it

* Only try to download an update if it's available

* Remove separate `will-quit` handler

* Clarify comment

* Invert condition in `preventInstallingOutdatedUpdates`

* Add comments

* Move `closeAllConnections()` out of try catch

* Add specialized functions to serialize/deserialize errors

* Add tests

* Leave TODO

* Do not wait for `maybeRemoveAppUpdatesManagingCluster` in `useClusterLogout`

* Hide app updater for .tar.gz which is not supported

* Add missing mock

(cherry picked from commit 13aa750)

* Show update widget in login dialog (#57695)

* Add app updates widget to login dialog

* Adjust compatibility warning to app updates

* Simplify widget error state, do not show full error message

* Make the widget take up 100% width both in login modal an in story

* Fix LocalWithPasswordless story

* Use `alignItems="stretch"`

* Use `SpaceProps` instead of redefining `mx` manually

* Infer type

* Display buttons in the bottom of Alert

* Avoid moving content when scrollbar appears

* Destructure `Promise.all` result

* Add comment

---------

Co-authored-by: Rafał Cieślak <rafal.cieslak@goteleport.com>
(cherry picked from commit c59a4e6)

* Simplify auto updates multi-cluster view (#57709)

* Simplify auto updates multi-cluster view

* Improve labels

* Fix tests

(cherry picked from commit b574045)

* Connect: Force focus after a successful SSO login

* Move login functions from ClustersService to useClusterLogin

* Remove no longer used GetResourcesParams

* Remove clusters/types.ts

* Force focus on Connect after successful SSO login

* Update SSO prompt after login but before sync

* Add test for SSO prompts

* Use `Promise.withResolvers`

Co-authored-by: Grzegorz Zdunek <gzdunek@users.noreply.github.com>

* Update wait-for-sync prompt message

---------

Co-authored-by: Grzegorz Zdunek <gzdunek@users.noreply.github.com>

* Hardcode the lowest supported Connect versions that support managed updates (#58348)

* Provide the lowest supported app versions for managed updates

* Deserialize errors in the UI code, not in preload

Otherwise, the error name is lost.

* Bump version in tests

* Lint

* Show exact version in error message

* Do not hide UnsupportedVersionError

* Fix margin

* Make alert take 100% width

(cherry picked from commit d7e1352)

---------

Co-authored-by: Rafał Cieślak <rafal.cieslak@goteleport.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ravicious ravicious ravicious approved these changes

@ryanclark ryanclark ryanclark approved these changes

Assignees

No one assigned

Labels

backport/branch/v17 backport/branch/v18 no-changelog Indicates that a PR does not require a changelog entry size/sm ui

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@gzdunek @ravicious @ryanclark