[BACK-4437] Address login-theme design-review feedback#55
Open
toddkazakov wants to merge 15 commits into
Open
Conversation
Design review asked for a tighter 32px rhythm between card sections. Scale the composed gaps with it (16px row gap + 16px extra before action rows) and the negative margins that carve tight 16px stacks out of the section gap (OTP hint/banner, TOTP verify fields). The reset-password rule's comment claimed a 12px margin produced the 48px gap; the real total was 24px PF grid gap + 12px intrinsic PF actions offset + 12px margin, so the margin becomes -4px to land on 32px. Also give the logout-confirm button the same 32px section gap — it previously sat flush against the prose. Page chrome (page padding, logo-to-card gap) and the recovery-codes print stylesheet intentionally keep their existing values.
Design review: the code input now comes before the device-name field (step-entry focus follows it), and the recommended-apps list renders disc bullets instead of a plain indented list.
- Unknown email on the username step reads "This email doesn't belong to an account yet." again (carried over from the legacy theme, overriding the home-idp-discovery SPI's bundled "Unknown email."). - Recovery-codes warning banner: "Save these codes and store them in a safe place separate from your phone..." with the you-can't-return caveat as the body line. - Forgot-password subtitle now says "at the following email" and the post-submit success alert is rephrased around "A password reset link has been sent to your email."
Shared completionGate script (register-commons.ftl) keeps the submit button disabled until every required-marked field is filled and, on the clinician form, the terms checkbox is checked. Required fields are detected via the label's required-asterisk span since field.ftl never emits the HTML required attribute. The button is only disabled from JS, so no-JS submissions still reach server-side validation.
"Need help setting up your authenticator app? Visit our support documentation." — replaces "Asked for additional details?..." on both the TOTP setup and recovery-codes screens (shared message key).
A 2FA device label can be a single unbroken string up to 200 chars; nothing allowed mid-word breaks, so the delete-credential message crept out of the card (design review A.1, visible at 150% zoom). Add overflow-wrap: anywhere to every surface that renders the label — the page title (previously saved only by PF's .pf-v5-c-title word-break), the delete-credential/idp-link message block, the OTP device hint, and the multi-credential picker rows.
Expired password-reset / email-verification / update-email links used to surface as a generic "Action expired" alert on whatever page the flow restarted to. Detect the expiry (tp-commons isActionLinkExpired — Keycloak uses expiredActionTokenSessionExistsMessage on the flow-restart path, ...NoSessionMessage on the sessionless error page, and expiredActionMessage for required-action expiry) and render the Figma 14515:80071 card instead: "This link is no longer active" with a "Take me back to the application" button (client baseUrl, falling back to restarting the login flow). Wired into login-username.ftl (session path) and error.ftl (no-session path).
The No/Yes labels are too long to share a row on narrow viewports — the Yes button overflowed the card. At the theme's 767px breakpoint the pair now stacks full-width with "Yes, trust this device" first; desktop keeps the side-by-side No | Yes order from the Figma frame.
Match the "Confirmation email sent" info page to Figma 14078:24389: bold the submitted address, break before the second sentence, and say "Click the link to complete the update and activate your new email address" instead of "You need to confirm via email...". info.ftl now renders message.summary through kcSanitize(...)?no_esc (the pattern error.ftl and the template.ftl alert already use) so the message can carry the markup; sanitized plain-text messages on the other info screens render unchanged.
Match the alert on the update-email resend/cancel gate to Figma
14177:33676: "A verification email has been sent to <address>. Please
verify your email address to complete the update and continue
accessing your account." plus a second line pointing at resend and
Cancel. The address renders as a bold mailto link per the frame
(kcSanitize keeps the anchor), and the apostrophe in "Didn't" is
doubled because MessageFormat processes the {0} parameter.
Implicit submission (Enter in a text field) clicks the form's first submit button, and our action rows put Cancel before the primary to match the Figma left-to-right order — upstream renders the primary first, so this is theme-introduced. Enter cancelled the email change on update-email (both cancel branches), cancelled the AIA password update, and on the TOTP setup form cancelled the whole setup from the step-2 code/label fields while bypassing validation (the submit listener skips validation for the cancel-aia submitter). Add a visually-hidden unnamed default submit as the first submit in each affected form (.tp-default-submit — off-screen rather than display:none so every browser treats it as the default button). The stub posts exactly what clicking the primary posts, visuals and tab order are untouched, and reordering wouldn't have fixed the TOTP form anyway since its Cancel lives in step 1 ahead of the step-2 fields.
Match the iPhone SE frame (Figma 14642:38389): at the theme's 767px breakpoint the page scaffold gets 32px/24px padding and a 24px logo-to-card gap instead of the desktop 48px 16px 32px and 48px. The card's own 32px padding and the 32px section rhythm inside it are the same on both frames, so only the scaffold changes.
The code list is a two-column max-content grid (~390px wide) that overflowed the 327px card on a 375px screen. Per the mobile frame (Figma 14647:38700), at the 767px breakpoint the codes stack in one centered column — groups 1-6 and 7-12 separated by the frame's 44px — and the tinted box spans the full card width instead of shrink-wrapping to the code column. Desktop keeps the side-by-side layout.
Unlike the other cancel-bearing forms, the confirmation checkbox lives inside this form, and in the AIA branch the visible Cancel is the first submit button. Browsers that run implicit submission from a focused checkbox (Firefox does, Chromium doesn't) would cancel the whole setup on Enter. Add the tp-default-submit stub as the form's first submit, with its disabled state toggled by the same checkbox handler that gates Finish: while unchecked the default button is disabled, which makes implicit submission a no-op instead of letting Enter bypass the confirmation gate; once checked, Enter finishes (the stub is unnamed, like Finish, so the POST is identical).
The updated frame (Figma 14641:35974) shows "No, require verification each time" above the primary Yes on mobile — drop the order: -1 that forced the primary on top so the pair stacks in plain DOM order. Desktop keeps the side-by-side No | Yes layout.
toddkazakov
changed the title
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
1 participant
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.
Design-review fixes on top of the theme redesign (#54):
📚 Stacked PR 9 of 9 (final). Base branch:
tk-stack-8-theme-redesign— review/merge it first.Stack — review & merge bottom-up:
tk-stack-1-trusted-device-spi→mastertk-stack-2-attempted-username-fix→tk-stack-1-trusted-device-spitk-stack-3-aia-authenticator→tk-stack-2-attempted-username-fixtk-stack-4-2fa-cleanup-listeners→tk-stack-3-aia-authenticatortk-stack-5-login-activity-outbox→tk-stack-4-2fa-cleanup-listenerstk-stack-6-email-changed-notification→tk-stack-5-login-activity-outboxtk-stack-7-cancelable-email-change→tk-stack-6-email-changed-notificationtk-stack-8-theme-redesign→tk-stack-7-cancelable-email-changetk-stack-9-design-review-fixes→tk-stack-8-theme-redesignTheme-only diff (FTL / CSS / message properties — no Java). Flows verified live against the local dev1 realm.