Show update widget in login dialog by gzdunek · Pull Request #57695 · gravitational/teleport

gzdunek · 2025-08-08T09:57:31Z

Adds showing the update widget in the login dialog:

gzdunek · 2025-08-08T09:57:53Z

+        maxWidth="432px"
+        css={`
+          > * {
+            width: 100%;


I couldn't get it right, so I added this little workaround.
If I set 100% width on AvailableUpdate in Widget.tsx then the widget takes full width in the AppUpdater story, but overflows the LoginDialog. If I remove it, then it takes full width in LoginDialog, but in the AppUpdater story it's too narrow.
I couldn't find a source of this difference.

Does alignItems="stretch" work?

That solves it, thanks!

ryanclark · 2025-08-08T10:04:53Z

+        maxWidth="432px"
+        css={`
+          > * {
+            width: 100%;


Does alignItems="stretch" work?

ryanclark · 2025-08-08T10:19:05Z

  updateEvent: AppUpdateEvent;
  platform: Platform;
  clusterGetter: ClusterGetter;
+  mx?: string | number;


Might be overkill for this but in Identity Security when I want to pass style properties to children components I try to extend either the props of the child component (BoxProps, etc) or the style props (MarginProps in this case), collecting them in ...rest and passing {...rest} down, so I'm not redefining style types and it is more flexible in the future

Just a thought, feel free to ignore

Makes sense, done.
I forgot that we have better options to pass style props than [x: string]: any; :p

ryanclark · 2025-08-08T10:21:09Z

+  props: ClusterLoginPresentationProps
+) => {
+  return (
+    <StepSlider<typeof loginViews>


Does this type not get inferred because of flows={loginViews}?

I'm not sure why we're writing it, the type is inferred correctly.

ryanclark · 2025-08-08T10:22:35Z

 }

+const AppUpdateDetails = ({
+  refCallback,


Sorry if I'm missing something (I can't see where refCallback is used), how comes we use refCallback instead of forwardRef (or just the ref arg with React 19)?

Honestly, I don't know too why StepSlider doesn't use ref 🙈

I think it was probably just based on some of the older components that we've had like Modal or Popover. StepSlider itself was introduced mid 2022.

ryanclark · 2025-08-08T10:24:32Z

+      findCluster: (clusterUri: RootClusterUri) =>
+        clustersService.findCluster(clusterUri),


Could this be

Suggested change

findCluster: (clusterUri: RootClusterUri) =>

clustersService.findCluster(clusterUri),

findCluster: clustersService.findCluster,

findCluster is a method in ClustersService class, so I'd need to bind this. I think the arrow function is cleaner.

ravicious · 2025-08-08T15:49:07Z

I'm not sure if this multidimensional navigation is the best solution here. I think maybe the biggest problem is that going from the main login method to the update details and then back resets the original login slider? Like in the recording where I go from local auth to SSO options then to the update details, but when I go back from there I'm on local auth instead of SSO options.

multiple-sliders.mov

I considered a few options for what should happen on "more" click:

Open a regular app update dialog - this would override the existing dialog. Probably the worst option.

Open an important update dialog - but those are only opened from tshd, we shouldn't make an exception for this.

Expand the widget into a detailed view - in a multi-cluster setup the screen would become cluttered and hard to follow.

That leaves the step slider as the most reasonable option IMO.

Now, the question is whether we should have a single or two-level flow.

My concern with the latter is complexity. Right now, the login form is independent from the app update details view. If we start merging them into a single slider, a few problems can happen:

We will need to conditionally show or hide elements outside of the slider, depending on the step. Login errors and the updater widget are displayed outside the loginViews slider so they're always visible. In the update details view, they shouldn't be. (won't hiding them cause the content to jump?)

We will need different flows depending on auth options: { default: [Primary, Secondary, AppUpdates] } vs { default: [Primary, AppUpdates] }. This adds to the complexity.

It seems to me that the single-slider option requires quite a lot of work for not much benefit.
Resetting the child step slider is slightly annoying, but I'm not sure how often users actually run into this in practice.

We will need to conditionally show or hide elements outside of the slider, depending on the step. (…)

I was imagining that the update widget would be shown only in Primary, Secondary wouldn't have anything related to updates. This way we pull everything within some slider step, doesn't this solve the problem of conditionally showing stuff?

We will need different flows depending on auth options: { default: [Primary, Secondary, AppUpdates] } vs { default: [Primary, AppUpdates] }. This adds to the complexity.

Would that be the case though? At the moment, the slider used in FormLogin.tsx has loginViews hardcoded to [Primary, Secondary] regardless of auth options, so couldn't we add AppUpdates as the third element?

In any case, I agree that it probably won't happen often that regular users see this.

I was imagining that the update widget would be shown only in Primary, Secondary wouldn't have anything related to updates. This way we pull everything within some slider step, doesn't this solve the problem of conditionally showing stuff?

Yeah, it would partially solve the problem. We also have a login error displayed above the slider, but it could be moved into Primary and Secondary screens (or shown conditionally).

Would that be the case though? At the moment, the slider used in FormLogin.tsx has loginViews hardcoded to [Primary, Secondary] regardless of auth options, so couldn't we add AppUpdates as the third element?

Right now, you can't see the Secondary view if auth options don't allow it, since the Primary view has no button for it.
But with AppUpdates as the third view, we'd need a way to jump straight from the first view to the third. I'm not sure how to do this with StepSlider, since it only provides a next() function (would we have to call it twice?). Even with something like goToStep(), I think you'd still have to pass through the second step (but I'm not a StepSlider expert 🙂)

In any case, I agree that it probably won't happen often that regular users see this.

👍

ravicious · 2025-08-08T15:51:58Z

This effect where the scrollbar sets in and affects the width of the content in the modal is not great. But if your experience with the StepSlider are going to be anything like mine when I was adding the VNet panel, I'd say we're just not going to address this before the v1 release. 😅 We're reaching the limits of what the slider was created for.

Maybe one of those esoteric styles that affect whether the scrollbar counts as box width or not would help here?

scrollbar-after-animation.mov

I think maybe the bigger problem is that the scrollbar gets added not on the whole modal but rather just on the "middle" container with content.

For example, if you take the job role modal, you can see that the whole modal gets overflow rather than just the middle part.

Job role modal

user-job-role-modal.mov

With how StepSlider works though I think it might be unavoidable? Unless the whole login modal would be one big slider like it's currently on master.

Tbh maybe it wouldn't be that bad? This would also get rid of the problem with multidimensional navigation, because you'd either be able to look at the main login option, the other sign-in options, or the update details.

This effect where the scrollbar sets in and affects the width of the content in the modal is not great.

I managed to overcome this by disabling shrinking of the slider content, so the scroll bar is now displayed on the dialog level. The issue is almost invisible now.

Tbh maybe it wouldn't be that bad? This would also get rid of the problem with multidimensional navigation, because you'd either be able to look at the main login option, the other sign-in options, or the update details.

I responded to that in #57695 (comment).

I managed to overcome this by disabling shrinking of the slider content, so the scroll bar is now displayed on the dialog level. The issue is almost invisible now.

What do you mean by "almost"? 😅 It seems to work good now!

I noticed that the text was wrapping to the next line during the transition (for a fraction of a second), but I can't reproduce it anymore. It seems to work good indeed!

ravicious · 2025-08-18T17:10:06Z

We will need to conditionally show or hide elements outside of the slider, depending on the step. (…)

I was imagining that the update widget would be shown only in Primary, Secondary wouldn't have anything related to updates. This way we pull everything within some slider step, doesn't this solve the problem of conditionally showing stuff?

We will need different flows depending on auth options: { default: [Primary, Secondary, AppUpdates] } vs { default: [Primary, AppUpdates] }. This adds to the complexity.

Would that be the case though? At the moment, the slider used in FormLogin.tsx has loginViews hardcoded to [Primary, Secondary] regardless of auth options, so couldn't we add AppUpdates as the third element?

In any case, I agree that it probably won't happen often that regular users see this.

ravicious · 2025-08-18T17:14:01Z

I managed to overcome this by disabling shrinking of the slider content, so the scroll bar is now displayed on the dialog level. The issue is almost invisible now.

What do you mean by "almost"? 😅 It seems to work good now!

ravicious · 2025-08-18T17:19:00Z

 }

+const AppUpdateDetails = ({
+  refCallback,


I think it was probably just based on some of the older components that we've had like Modal or Popover. StepSlider itself was introduced mid 2022.

ravicious · 2025-08-18T17:22:30Z

+  const [initAttempt, init] = useAsync(() => {
+    return Promise.all([
+      tshd.getAuthSettings({ clusterUri }).then(({ response }) => response),
+      mainProcessClient.checkForAppUpdates(),


IIRC, mainProcessClient.checkForAppUpdates() is not expected to return a rejected promise and instead surfaces the error in some other way. It'd be good to leave a comment about it here.

backport-bot-workflows · 2025-08-19T09:41:40Z

@gzdunek See the table below for backport results.

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

* 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>

* 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)

* 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>

gzdunek added 4 commits August 8, 2025 10:39

Add app updates widget to login dialog

7f10067

Adjust compatibility warning to app updates

c41be2e

Simplify widget error state, do not show full error message

70f52cd

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

5cd4519

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

gzdunek commented Aug 8, 2025

View reviewed changes

github-actions bot added size/md ui labels Aug 8, 2025

github-actions bot requested review from flyinghermit and kimlisa August 8, 2025 09:58

gzdunek requested review from ravicious and ryanclark and removed request for flyinghermit and kimlisa August 8, 2025 09:58

ryanclark reviewed Aug 8, 2025

View reviewed changes

Fix LocalWithPasswordless story

ce4759d

ravicious reviewed Aug 8, 2025

View reviewed changes

ravicious self-requested a review August 11, 2025 14:52

gzdunek added 6 commits August 18, 2025 11:32

Use alignItems="stretch"

e7c4024

Use SpaceProps instead of redefining mx manually

ff2ef84

Infer type

c0d570e

Display buttons in the bottom of Alert

cba7dbc

Avoid moving content when scrollbar appears

d018456

Destructure Promise.all result

18df3b7

gzdunek requested a review from ryanclark August 18, 2025 12:15

ryanclark approved these changes Aug 18, 2025

View reviewed changes

ravicious approved these changes Aug 18, 2025

View reviewed changes

Add comment

aea46d4

gzdunek added this pull request to the merge queue Aug 19, 2025

Merged via the queue into master with commit c59a4e6 Aug 19, 2025
40 checks passed

gzdunek deleted the gzdunek/update-widget branch August 19, 2025 09:39

This was referenced Aug 22, 2025

[v18] Show app updater in UI #58260

Merged

[v17] Show app updater in UI #58261

Merged

		findCluster: (clusterUri: RootClusterUri) =>
		clustersService.findCluster(clusterUri),

	findCluster: (clusterUri: RootClusterUri) =>
	clustersService.findCluster(clusterUri),
	findCluster: clustersService.findCluster,

Conversation

gzdunek commented Aug 8, 2025

Uh oh!

gzdunek Aug 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gzdunek Aug 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ravicious Aug 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

backport-bot-workflows bot commented Aug 19, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gzdunek Aug 8, 2025 •

edited

Loading

gzdunek Aug 18, 2025 •

edited

Loading

ravicious Aug 8, 2025 •

edited

Loading