Expose reusable settings UI for add-ons#95
Conversation
… style)
Expose Texty's schema-driven settings UI so add-ons (Texty Pro) can import it
instead of duplicating the renderer:
- Build a `components` webpack entry from src/components/index.tsx as a window
library `textyComponents` (handle `texty-components`), and wire a custom
DependencyExtractionWebpackPlugin via webpack-dependency-mapping.js so
`@texty/components` externalizes to that global + handle.
- Register the `texty-components` script in Admin/Menu so dependents load it.
- Generalize NotificationGroupSettings: optional schemaPath/savePath props and a
generic save-fold (collapse any `<id>_<key>` / `<id>::<key>` children into the
per-id payload), so it renders any feature's {schema, values} — not just the
built-in notifications groups. Exported as `SchemaSettings`.
Use the nested window.texty.components global (Dokan style) instead of a flat
textyComponents. The library assignment is merge-safe
((window.texty = window.texty || {}).components = …), and texty-components now
depends on texty-admin so it loads after the localized `var texty = {…}` data and
merges onto it rather than being clobbered.
…uery args - Apply `texty_notification_icon_map` / `texty_notification_logo_map` JS filters to the header ICON_MAP / LOGO_MAP so add-ons can register icons/logos for their own groups. - Build REST query strings with @wordpress/url's addQueryArgs instead of manual concatenation / encodeURIComponent (schema?group, notifications?context).
|
Warning Review limit reached
Next review available in: 47 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
📝 WalkthroughWalkthroughIntroduces a shared ChangesAdd-on extensibility infrastructure and notification refactoring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
kzamanbd
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/pages/notifications/components/NotificationGroupSettings.tsx`:
- Around line 55-63: The Settings form key is currently derived only from
groupId in NotificationGroupSettings, so custom consumers using
schemaPath/savePath without a groupId can reuse the same instance and leak form
state between endpoints. Update the keying logic in NotificationGroupSettings to
include the resolved endpoint identity from fetchPath and postPath, and keep the
existing groupId-based key only as part of that composite so different
schemas/save targets mount distinct Settings instances.
- Around line 42-49: Make NotificationGroupSettings’ public props contract
mutually exclusive so invalid combinations cannot compile: in
NotificationGroupSettings, replace the current Props shape with a union that
allows either the built-in groupId flow or the custom endpoint flow, and require
schemaPath and savePath together for the custom case. Update the component’s
prop typing and any related call sites/helpers in NotificationGroupSettings so
<NotificationGroupSettings schemaPath="..." /> and bare
<NotificationGroupSettings /> are rejected unless they provide the correct
paired fields.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2855e4f2-edb2-4a6d-8abf-87a340aab108
⛔ Files ignored due to path filters (10)
assets/images/gateways/clickatell-logo.pngis excluded by!**/*.pngassets/images/gateways/clickatell.svgis excluded by!**/*.svgassets/images/gateways/common-sms.pngis excluded by!**/*.pngassets/images/gateways/plivo-logo.pngis excluded by!**/*.pngassets/images/gateways/plivo.svgis excluded by!**/*.svgassets/images/gateways/twilio-logo.pngis excluded by!**/*.pngassets/images/gateways/twilio.svgis excluded by!**/*.svgassets/images/gateways/vonage-logo.pngis excluded by!**/*.pngassets/images/gateways/vonage.svgis excluded by!**/*.svgpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (12)
includes/Admin/Menu.phpincludes/Gateways/Clickatell.phpincludes/Gateways/Fake.phpincludes/Gateways/Plivo.phpincludes/Gateways/Twilio.phpincludes/Gateways/Vonage.phpsrc/components/index.tsxsrc/pages/notifications/components/NotificationGroupSettings.tsxsrc/pages/notifications/hooks/useNotifications.tssrc/pages/notifications/index.tsxwebpack-dependency-mapping.jswebpack.config.js
| type Props = { | ||
| groupId: string; | ||
| // Built-in notifications group (renders /notifications/schema?group=…). | ||
| groupId?: string; | ||
| // Or point at any endpoint returning { schema, values } (e.g. Texty Pro's | ||
| // Automations), with a matching save endpoint. | ||
| schemaPath?: string; | ||
| savePath?: string; | ||
| }; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Make the public props contract impossible to misuse.
schemaPath and savePath are optional independently here, so <NotificationGroupSettings schemaPath="/foo" /> still saves to /texty/v1/notifications, and <NotificationGroupSettings /> fetches /texty/v1/notifications/schema?group=. Since src/components/index.tsx now exports this as add-on API, these invalid combinations should be rejected up front instead of failing against the wrong endpoint.
Proposed fix
-type Props = {
- // Built-in notifications group (renders /notifications/schema?group=…).
- groupId?: string;
- // Or point at any endpoint returning { schema, values } (e.g. Texty Pro's
- // Automations), with a matching save endpoint.
- schemaPath?: string;
- savePath?: string;
-};
+type Props =
+ | {
+ // Built-in notifications group (renders /notifications/schema?group=…).
+ groupId: string;
+ schemaPath?: never;
+ savePath?: never;
+ }
+ | {
+ // Custom feature endpoints must be provided as a pair.
+ groupId?: never;
+ schemaPath: string;
+ savePath: string;
+ };Also applies to: 60-63, 155-157
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/notifications/components/NotificationGroupSettings.tsx` around
lines 42 - 49, Make NotificationGroupSettings’ public props contract mutually
exclusive so invalid combinations cannot compile: in NotificationGroupSettings,
replace the current Props shape with a union that allows either the built-in
groupId flow or the custom endpoint flow, and require schemaPath and savePath
together for the custom case. Update the component’s prop typing and any related
call sites/helpers in NotificationGroupSettings so <NotificationGroupSettings
schemaPath="..." /> and bare <NotificationGroupSettings /> are rejected unless
they provide the correct paired fields.
| const NotificationGroupSettings = ({ | ||
| groupId, | ||
| schemaPath, | ||
| savePath, | ||
| }: Props) => { | ||
| const fetchPath = | ||
| schemaPath ?? | ||
| addQueryArgs('/texty/v1/notifications/schema', { group: groupId ?? '' }); | ||
| const postPath = savePath ?? '/texty/v1/notifications'; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Key the form by endpoint, not only by group ID.
After this was generalized, every custom consumer that passes schemaPath/savePath without a groupId gets key={undefined}. Switching between two custom schemas will reuse the same <Settings> instance and carry its internal dirty/form state across datasets.
Proposed fix
- key={groupId}
+ key={fetchPath}Also applies to: 215-216
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/notifications/components/NotificationGroupSettings.tsx` around
lines 55 - 63, The Settings form key is currently derived only from groupId in
NotificationGroupSettings, so custom consumers using schemaPath/savePath without
a groupId can reuse the same instance and leak form state between endpoints.
Update the keying logic in NotificationGroupSettings to include the resolved
endpoint identity from fetchPath and postPath, and keep the existing
groupId-based key only as part of that composite so different schemas/save
targets mount distinct Settings instances.
1 participant
Summary
Lets add-ons (e.g. Texty Pro) reuse Texty's schema-driven settings UI instead of
duplicating it, using the same webpack dependency-extraction pattern as Dokan
lite → pro. Underpins the Texty Pro SMS Automations PR.
What's included
Component export (webpack library + dependency extraction)
componentswebpack entry →dist/components.jsexposingwindow.texty.components(handletexty-components).webpack-dependency-mapping.js(requestToExternal/requestToHandle) wired to acustom
DependencyExtractionWebpackPlugin, so@texty/componentsexternalizes tothe global + script handle.
src/components/index.tsxbarrel exportsNotificationGroupSettingsandPhoneField.Admin/Menuregisterstexty-components(depends ontexty-adminso it mergesonto
window.textyafter the localized data).Generalized
NotificationGroupSettingsschemaPath/savePathprops + a generic save-fold (collapse any<id>_<key>/<id>::<key>children into the per-id payload) → renders anyfeature's
{ schema, values }, not just built-in notification groups.Extensibility / cleanup
ICON_MAP/LOGO_MAPmade filterable viatexty_notification_icon_map/texty_notification_logo_map.@wordpress/urladdQueryArgs.Consumer
Add-ons:
import { NotificationGroupSettings, PhoneField } from '@texty/components'.Test
npm run build(tsc clean); User Events / Integrations tabs render unchanged;the components bundle exposes
window.texty.components.Summary by CodeRabbit
New Features
Bug Fixes