feat(admin): Add widgets in dashboard in AdminCP - #730
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a3bf82a603
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const managedIds = saved | ||
| .filter(item => widgetIds.has(widgetIdOf(item.id))) | ||
| .map(item => item.id); |
There was a problem hiding this comment.
Include default widgets in the managed ID set
On a fresh dashboard, saved is empty while normalizeLayout adds the defaultEnabled widgets, so none of those visible cards enter managedIds. If the admin removes one and saves, mergeLayoutForSave receives no previous or managed entry from which to create { hidden: true }; the next load therefore treats the widget as unseen and adds it back. Include the initially normalized layout IDs in the managed set so a default widget can be removed on the first edit.
Useful? React with 👍 / 👎.
| export const getDashboardLayoutAdminRoute = buildRoute({ | ||
| pluginId: CONFIG_PLUGIN.pluginId, |
There was a problem hiding this comment.
Add staff permission gates to the dashboard APIs
This route, along with save-layout.route.ts and save-widget-settings.route.ts, omits adminStaffPermission; checking c.get("admin") only authenticates an admin and does not run the repository's fine-grained staff authorization middleware. Consequently these new endpoints cannot be restricted for limited staff accounts as required; assign appropriate module permissions to all three route definitions.
AGENTS.md reference: AGENTS.md:L22-L24
Useful? React with 👍 / 👎.
| <Settings settings={item.settings ?? {}} widgetId={item.id} /> | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
Defer settings components until their dialogs open
When a plugin's settingsComponent performs expensive rendering or database work, instantiating it here executes that work on every dashboard request even though the settings dialog is closed; the catalog block also eagerly renders stand-ins for unplaced and repeatable widgets. Put this content behind the required lazy/Suspense dialog boundary so ordinary dashboard loads do not pay for every registered settings form.
AGENTS.md reference: AGENTS.md:L10-L10
Useful? React with 👍 / 👎.
| return ( | ||
| <form | ||
| className="flex flex-col gap-6" | ||
| onSubmit={event => { |
There was a problem hiding this comment.
Replace the hand-built settings form with AutoForm
This newly introduced settings UI manually manages form state, labels, submission, and validation instead of using the repository-standard AutoForm. Converting it to AutoForm keeps settings forms on the shared validation and accessibility path required by the repository convention.
AGENTS.md reference: AGENTS.md:L9-L9
Useful? React with 👍 / 👎.
| const { widgetId, settings } = c.req.valid("json"); | ||
| if (isSettingsTooLarge(settings)) { | ||
| throw new HTTPException(413, { message: "Widget settings too large" }); |
There was a problem hiding this comment.
Enforce the settings cap after merging the patch
When a widget already has settings, this checks only the incoming patch, but mergeWidgetSettings later spreads that patch over the existing bag. Two requests containing different keys of roughly 40 KB each therefore both pass while persisting a result larger than the advertised 64 KB cap, and repeated patches can grow the JSON without a meaningful bound. Validate the fully merged settings inside the transaction before updating the row.
Useful? React with 👍 / 👎.
Improving Documentation
pnpm lint:fixto fix formatting issues before opening the PR.Description
What?
Why?