Repository navigation
⏳ fix: Restore Login Card Width and Show SSO Loading State - #123
Closed
dustinhealy wants to merge 4 commits into
Closed
dustinhealy wants to merge 4 commits into
dustinhealy wants to merge 4 commits into
Conversation
Inline @clickhouse/click-ui in vitest server deps and exclude .claude/** from test discovery. click-ui 0.9.1 ships ESM that imports .css files, which Node's loader cannot parse, so Vitest must process the package through Vite.
click-ui 0.9.1 ships unlayered CSS while Tailwind utilities live in @layer utilities, so Panel's new width: var(--panel-width, auto) rule beat our w-full class and the login card collapsed to content width. Both Panels now use the fillWidth prop, which sets an inline --panel-width: 100% that wins the cascade, and the dead w-full class is replaced with min-w-70 so the card can never collapse below 17.5rem again even if the component CSS changes. The SSO button never showed a loading indicator because handleSsoLogin reset ssoLoading in a finally block that ran immediately after the window.location.href assignment, which does not block, so the state reverted before the redirect painted. The success path now returns with ssoLoading still true and only error paths reset it, and the button uses click-ui's loading prop for the spinner, disabled state, and aria-busy. The same finally reset is removed from the auto-redirect effect. Adds AuthCard tests covering the pending, redirecting, and error states of the SSO button plus a regression guard asserting both Panels carry the fillWidth inline style and min width class.
dustinhealy
changed the title
dustinhealy
marked this pull request as ready for review
August 11, 2026 23:03
Contributor
Author
|
@codex review |
dustinhealy
marked this pull request as draft
August 14, 2026 15:04
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f7d882db48
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The successful SSO path intentionally leaves ssoLoading true while the browser navigates to the identity provider, but browsers that restore the login page from the back-forward cache (for example after cancelling at the IdP via the Back button) also restore that React state, leaving the button stuck loading and disabled until a manual reload. A pageshow listener now clears the loading flag when the event reports a persisted page, keeping the loading state intact during the outbound navigation while recovering it on bfcache restores.
Frontend component tests belong in an adjacent __tests__ directory per the repo convention in AGENTS.md, and this file was added directly under src/components in this branch.
Contributor
Author
|
@codex re-review |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
dustinhealy
marked this pull request as ready for review
August 14, 2026 16:07
danny-avila
added a commit
that referenced
this pull request
Oct 28, 2026
… the Provider Loads (#196) * 🔑 feat: Add Google and SAML Admin Sign-In Through a Provider Registry and Keep Deep Links Through SSO The sign-in page offers one button per SSO provider LibreChat has for admins, from a registry in src/constants (SSO_PROVIDERS: OpenID, Google, SAML). ssoProvidersFn replaces checkOpenIdFn and returns { providers, ssoOnly }. OpenID still needs its /api/admin/oauth/openid/check probe. Google and SAML come from one GET /api/config sent with the request's tenant, and count only with socialLoginEnabled, since LibreChat registers their admin strategies under ALLOW_SOCIAL_LOGIN alone. A provider left out of socialLogins is not offered, as on LibreChat's own sign-in page. ADMIN_SSO_ENABLED=false still offers none. OpenID and SAML buttons take the deployer's label from /api/config. One POST ssoStartFn ({ provider, redirectTo }) builds the PKCE challenge and keeps the verifier and the sanitized in-app path (toSafeRedirect) in the admin session, instead of the redirectTo query LibreChat strips. oauthExchangeFn takes { code, provider }, stores the provider as tokenProvider, and returns and then forgets the path. One /auth/$provider/callback route replaces the OpenID-only one: an unknown provider is not found, a failure shows SsoError, and a sign-in returns to the page it started from. Both callback paths are known metrics routes. Google sessions refresh through POST /api/admin/oauth/refresh with provider: 'google' and keep the rotated refresh token. SAML sessions, which LibreChat cannot renew, are never sent to the cookie-based /api/auth/refresh. ADMIN_SSO_ONLY auto-redirects only when exactly one provider is offered, hides the password form while any is, and brings the form back when a provider cannot start or none resolves. Supersedes #36 by @dustinhealy, reusing its provider registry, its /api/config-based availability with the social-login and socialLogins gates, and its idea of carrying the deep link through the admin session. Closes #30 Closes #65 * 🔗 fix: Bind the SSO Session to the Provider That Started It and Follow the socialLogins Order * 🧪 test: Expect the SSO Buttons in LibreChat's socialLogins Order * ⏳ fix: Restore the Sign-In Card Width and Keep SSO Buttons Busy Until the Provider Loads click-ui 0.9.1's unlayered `.panel { width: var(--panel-width, auto) }` outranks Tailwind's layered `w-full`, so the sign-in card and the SSO-only redirect card took their content's width: 304px and 311px at 1440 instead of max-w-md's 448px. Both Panels now take Panel's `fillWidth` (an inline `--panel-width: 100%`) and keep max-w-md. startSso reset its busy state in a `finally` that ran right after `window.location.href = authUrl`, so the button went back to its idle label, enabled, while the browser was still loading the provider, and could be clicked again. Only a start that fails gives the button back now: an error result, a request that throws, or a result with an empty URL (assigning it would reload /login). The button takes click-ui's `loading` (shimmer, disabled, aria-busy); the other providers stay disabled. On an SSO-only page with one provider, that state starts as the provider, so the redirect card shows from the first render and ends the same way. A `pageshow` with `persisted` (the browser restoring the page from its back-forward cache, Back from the provider) clears it: the button is idle again, and an SSO-only page offers its provider's button instead of a spinner that never ends. Supersedes #123 by @dustinhealy, whose fillWidth, loading and pageshow approach this follows. Depends on #188.
Contributor
|
Thanks @dustinhealy for this. #196 is now merged. It redoes this PR on top of #188's provider buttons and follows your approach throughout: Panel's What changed: Closing this in favor of #196. If the card or the loading state misbehaves somewhere, please comment or open an issue. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two login-screen fixes in
AuthCard.tsx:@layer utilities, and unlayered declarations beat layered ones regardless of specificity.Panelnow declareswidth: var(--panel-width, auto), which silently overrides ourw-full, so the card collapsed to content width (~248px). Fix: use the component'sfillWidthprop (inline--panel-width: 100%wins the cascade) on both Panels, drop the deadw-full, keepmax-w-md, and addmin-w-70(280px) so the card can never collapse below a usable width again.handleSsoLoginreset its loading flag in afinallythat runs immediately after thewindow.location.hrefassignment (assignment does not block), so no loading state ever painted, and the button never used click-ui'sloadingprop. Fix: keep the loading state through navigation (reset only on error paths) and drive the button withloading={ssoLoading}, which provides the spinner,disabled, andaria-busy. The auto-redirect effect gets the same treatment, and a response without anauthUrlnow surfaces the login-failed message instead of silently resetting.Heads-up for reviewers: the unlayered-vs-layered cascade means plain Tailwind geometry utilities (
w-*,p-*) no longer take effect on click-ui components as of 0.9.1; geometry should go through component props or important utilities (existing precedent:!max-w-2xlinFormDialog).Change Type
Testing
New
AuthCard.test.tsx(6 cases, real click-ui rendered): SSO button shows loading while pending, remains loading after resolving with an authUrl (the finally-bug regression test) withlocation.hrefset, returns to non-loading on error and on rejection, and both Panels render withfillWidthand the min width. 4 of 6 fail against the pre-fix code.Before/after proof:
Test Configuration:
Checklist