Expose reusable settings UI for add-ons#95
Open
kzamanbd wants to merge 16 commits into
Open
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: 30 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 selected for processing (2)
📝 WalkthroughWalkthroughAdds a shared ChangesAdd-on extensibility infrastructure and notification refactoring
Estimated code review effort: 3 (Moderate) | ~25 minutes 🚥 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
Add-ons that enqueue on wp-login.php (e.g. the registration OTP widget) declare a texty-components dependency, but the handle was only registered on admin_enqueue_scripts and wp_enqueue_scripts — never login_enqueue_scripts. On the registration form the script dep and its stylesheet went unresolved. Register on login_enqueue_scripts too.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/pages/notifications/index.tsx (1)
45-48: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winURL not corrected when
tabparam is invalid.When
requesteddoesn't match any tab, the UI falls back totabs[0], but thetabquery param stays at its invalid value — the URL and displayed tab diverge.🔧 Sync the URL to the resolved tab
+ useEffect(() => { + if (activeTab && requested !== activeTab) { + handleTabChange(activeTab); + } + }, [activeTab, requested]);🤖 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/index.tsx` around lines 45 - 48, The `activeTab` fallback in `src/pages/notifications/index.tsx` resolves invalid `requested` values to `tabs[0]`, but the `tab` query param is never updated, so the URL can diverge from the displayed tab. Update the logic around `requested`, `activeTab`, and `TAB_PARAM` so that when the param is missing or invalid, the URL is synchronized to the resolved tab value (using the existing navigation/router state in this page) instead of only falling back in render state.
🤖 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/index.tsx`:
- Around line 20-43: The tab list in Notifications is being frozen by useMemo
with an empty dependency array, so late-registered `texty_notifications_tabs`
entries never appear after the admin SPA mounts. Update `Notifications` to
derive the tabs from the current filter state on each render, or add the proper
dependencies so `applyFilters('texty_notifications_tabs', [])` can be
re-evaluated when extensions register; keep the logic around `useSearchParams`,
`useMemo`, and the `tabs` array in sync with dynamic add-on hooks.
---
Nitpick comments:
In `@src/pages/notifications/index.tsx`:
- Around line 45-48: The `activeTab` fallback in
`src/pages/notifications/index.tsx` resolves invalid `requested` values to
`tabs[0]`, but the `tab` query param is never updated, so the URL can diverge
from the displayed tab. Update the logic around `requested`, `activeTab`, and
`TAB_PARAM` so that when the param is missing or invalid, the URL is
synchronized to the resolved tab value (using the existing navigation/router
state in this page) instead of only falling back in render state.
🪄 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: 4ce4b5f7-70ee-4c6e-9b3d-9098b198e97b
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
includes/Admin/Menu.phpincludes/Assets.phpsrc/components/PhoneField.tsxsrc/components/index.tsxsrc/pages/notifications/components/NotificationGroupSettings.tsxsrc/pages/notifications/index.tsxtexty.php
✅ Files skipped from review due to trivial changes (1)
- includes/Admin/Menu.php
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/index.tsx
- src/pages/notifications/components/NotificationGroupSettings.tsx
…ogic in NotificationGroupSettings
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
Documentation