Show addon install popup when clicking to locked template#705
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #705 +/- ##
============================================
- Coverage 29.27% 27.58% -1.70%
+ Complexity 7266 6392 -874
============================================
Files 105 90 -15
Lines 23941 18850 -5091
============================================
- Hits 7009 5199 -1810
+ Misses 16932 13651 -3281 ☔ View full report in Codecov by Sentry. |
|
@truongwp I see an error when I try to test this.
It looks like it's trying to access (It looks like this is because I have an Elite license. It never includes the "upgrade" block in |
|
@truongwp lets start with just what is needed for quizzes here. Do we need anything besides the added category in the list, and the template Id? I think if it's just that, we can release it before quizzes even. Let's hold off on the rest of this so we can do it in a way that works more universally. |
|
@stephywells Do you mean we just need the quiz template and the category, we don't need the install Quizzes popup? |
|
@truongwp yes, exactly. For this PR, let's do it the same way the registration template works right now. |
|
@stephywells This is the code set icon for Quizzes category: https://github.com/Strategy11/formidable-forms/pull/705/files#diff-c3f25890259e06951f6e9c7f66838c489ff9c9e0462fd21978a96d8a3e869159R1283 Look like the category for template is set outside plugin, we will get them via API. Do we need to add Quizzes here? https://github.com/Strategy11/formidable-forms/blob/master/classes/controllers/FrmAddonsController.php#L118 |
|
@truongwp Yes, exactly. So we only need that line, and the line that adds the template id. But we should use a singular term so it matches the others (Quiz instead of Quizzes). The id for this template will be 28109851.
Sure! That would be great! |
WalkthroughThe pull request introduces several enhancements across multiple classes, primarily focusing on integrating addon requirements and improving template management. Key changes include the addition of methods to handle addon installation strings and the modification of existing methods to incorporate these new functionalities. The template listing logic is updated to reflect addon requirements, while the admin interface's JavaScript is enhanced for better user interaction and management of form actions. Overall, the changes aim to improve usability and maintainability within the application. Changes
Possibly related PRs
Suggested labels
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
js/formidable_admin.js (2)
Line range hint
8745-8952: Refactor to avoid usinghasOwnPropertydirectly on objectsThe code is using
hasOwnPropertydirectly on objects in a few places:if ( ! frm_admin_js.addonInstall.hasOwnProperty( addon ) ) { return; } if ( ! frm_admin_js.addonInstall.hasOwnProperty( 'upgrade' ) ) { // ... }It's recommended to use
Object.hasOwn()instead to avoid potential issues if an object has a property named "hasOwnProperty".Consider refactoring to:
if ( ! Object.hasOwn( frm_admin_js.addonInstall, addon ) ) { return; } if ( ! Object.hasOwn( frm_admin_js.addonInstall, 'upgrade' ) ) { // ... }This will make the code more robust and avoid relying on the
hasOwnPropertymethod of the object itself.🧰 Tools
🪛 Biome (1.9.4)
[error] 8898-8901: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.(lint/suspicious/noPrototypeBuiltins)
[error] 8914-8914: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.(lint/suspicious/noPrototypeBuiltins)
🪛 eslint
[error] 8897-8897: Identifier 'frm_admin_js' is not in camel case.
(camelcase)
[error] 8913-8913: Identifier 'frm_admin_js' is not in camel case.
(camelcase)
[error] 8923-8923: Identifier 'frm_admin_js' is not in camel case.
(camelcase)
[error] 8927-8927: Identifier 'frm_admin_js' is not in camel case.
(camelcase)
8897-8897: Use camelCase forfrm_admin_jsidentifierThe
frm_admin_jsidentifier is used in a few places but it's not following the camelCase naming convention. For consistency with the rest of the codebase, consider renaming it tofrmAdminJs.if ( ! frm_admin_js.addonInstall.hasOwnProperty( addon ) ) { return; } if ( ! frm_admin_js.addonInstall.hasOwnProperty( 'upgrade' ) ) { // ... } strings = frm_admin_js.addonInstall[ addon ]; strings.install_url = frm_admin_js.addonInstall.upgrade;After renaming:
if ( ! frmAdminJs.addonInstall.hasOwnProperty( addon ) ) { return; } if ( ! frmAdminJs.addonInstall.hasOwnProperty( 'upgrade' ) ) { // ... } strings = frmAdminJs.addonInstall[ addon ]; strings.install_url = frmAdminJs.addonInstall.upgrade;This will make the naming more consistent throughout the file.
Also applies to: 8913-8913, 8923-8923, 8927-8927
🧰 Tools
🪛 eslint
[error] 8897-8897: Identifier 'frm_admin_js' is not in camel case.
(camelcase)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
images/quizzes.pngis excluded by!**/*.png,!**/*.png
📒 Files selected for processing (5)
classes/controllers/FrmFormsController.php(2 hunks)classes/helpers/FrmAppHelper.php(2 hunks)classes/helpers/FrmFormsHelper.php(1 hunks)classes/views/frm-forms/list-template.php(2 hunks)js/formidable_admin.js(3 hunks)
👮 Files not reviewed due to content moderation or server errors (3)
- classes/views/frm-forms/list-template.php
- classes/helpers/FrmFormsHelper.php
- classes/controllers/FrmFormsController.php
🧰 Additional context used
🪛 Biome (1.9.4)
js/formidable_admin.js
[error] 8898-8901: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
[error] 8914-8914: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
🪛 eslint
js/formidable_admin.js
[error] 8897-8897: Identifier 'frm_admin_js' is not in camel case.
(camelcase)
[error] 8913-8913: Identifier 'frm_admin_js' is not in camel case.
(camelcase)
[error] 8923-8923: Identifier 'frm_admin_js' is not in camel case.
(camelcase)
[error] 8927-8927: Identifier 'frm_admin_js' is not in camel case.
(camelcase)
🔇 Additional comments (1)
classes/helpers/FrmAppHelper.php (1)
Line range hint 3126-3160: LGTM! Well-implemented dismissable warning message functionality
The new method provides a clean implementation for displaying and managing dismissable warning messages with proper:
- AJAX handling
- Nonce verification
- Permission checks
- Accessibility support
|
I'm going to close this for now for a couple reasons.
I figured out how to trigger this so I could get a screenshot of what it looked like. 
|
3 participants
Todo:
ADDONtext in the locked template.What I did:
'addon' => 'slug'to the template array.addon, show it as locked and adddata-addon="slug"to the `upgradepage of new form modal, check if<li>containsdata-addon, change all text/image/url of upgrade content into addon install content. The addon install content is registered in global JS variable.