gw-rich-text-html-fields.php: Added image upload capabilities.
#1075
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThis change enhances the rich text HTML field in Gravity Forms by integrating the WordPress media uploader into the TinyMCE editor within the form editor page. It enqueues the necessary media scripts and modifies the TinyMCE toolbar to include an image button. When this button is clicked, the WordPress media library is opened for image selection, and the selected image is inserted into the editor content as an Changes
Sequence Diagram(s)sequenceDiagram
participant AdminUser as Admin User
participant FormEditor as Form Editor Page
participant TinyMCE as TinyMCE Editor
participant WP_Media as WordPress Media Library
AdminUser->>FormEditor: Loads form editor page
FormEditor->>FormEditor: Enqueue wp_enqueue_media()
FormEditor->>TinyMCE: Initialize editor with image button
AdminUser->>TinyMCE: Clicks image button
TinyMCE->>WP_Media: Opens media library (image selection)
AdminUser->>WP_Media: Selects image
WP_Media->>TinyMCE: Returns image URL and alt text
TinyMCE->>FormEditor: Inserts <img> tag into editor content
Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
|
An alternate and easier fix could have been to include 'insert' on the toolbar (https://www.tiny.cloud/docs/tinymce/latest/quickbars/). Like so:
But it would have added other stuff along with insert image, not exactly as what David described. |
![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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
gravity-forms/gw-rich-text-html-fields.php(2 hunks)
🔇 Additional comments (2)
gravity-forms/gw-rich-text-html-fields.php (2)
19-19: Well done adding the media uploader.The addition of
wp_enqueue_media()is essential for the image upload functionality and is properly placed within the admin_init hook for the form editor page.
74-74: Good addition of the 'image' button to the toolbar.You've successfully extended the TinyMCE toolbar to include the image button, which is necessary for the new image upload functionality.
| // Handle image insertion from media library | ||
| editor.addButton( 'image', { | ||
| icon: 'image', | ||
| tooltip: 'Insert Image', | ||
| onclick: function() { | ||
| var frame = wp.media({ | ||
| title: 'Insert Media', | ||
| button: { text: 'Insert into HTML Field' }, | ||
| multiple: false, | ||
| library: { type: 'image' } | ||
| } ); | ||
|
|
||
| frame.on('select', function() { | ||
| var attachment = frame.state().get('selection').first().toJSON(); | ||
| editor.insertContent('<img src="' + attachment.url + '" alt="' + attachment.alt + '" />'); | ||
| } ); | ||
|
|
||
| frame.open(); | ||
| } | ||
| } ); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Image upload functionality looks good, but could use some enhancements.
The implementation correctly uses WordPress media API to handle image selection and insertion. The code appropriately:
- Opens the media library restricted to images
- Sets up the selection handler
- Inserts the selected image into the editor
A few suggestions for improvement:
Consider enhancing the implementation with:
frame.on('select', function() {
var attachment = frame.state().get('selection').first().toJSON();
- editor.insertContent('<img src="' + attachment.url + '" alt="' + attachment.alt + '" />');
+ // Escape attributes for security
+ var url = attachment.url.replace(/"/g, '"');
+ var alt = (attachment.alt || '').replace(/"/g, '"');
+
+ // Insert image with more attributes for better accessibility and control
+ editor.insertContent('<img src="' + url + '" alt="' + alt + '" class="size-full" />');
} );It might also be helpful to check if the TODO item related to image support should be removed from the file's header since it's now implemented:
🏁 Script executed:
#!/bin/bash
# Look for the TODO item related to image support
grep -n "Add support for inserting images" gravity-forms/gw-rich-text-html-fields.phpLength of output: 130
Image Upload Enhancement and Header Cleanup
-
The image insertion functionality using the WordPress media API is working well.
-
To further improve security and accessibility, update the image insertion code to escape attribute values and include an extra class. For example:
frame.on('select', function() { var attachment = frame.state().get('selection').first().toJSON(); - editor.insertContent('<img src="' + attachment.url + '" alt="' + attachment.alt + '" />'); + // Escape attributes for security + var url = attachment.url.replace(/"/g, '"'); + var alt = (attachment.alt || '').replace(/"/g, '"'); + // Insert image with enhanced accessibility and control + editor.insertContent('<img src="' + url + '" alt="' + alt + '" class="size-full" />'); } ); -
Additionally, the file header still contains the TODO "* 2. Add support for inserting images." (line 13). Since image support is now implemented, please remove this outdated comment.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Handle image insertion from media library | |
| editor.addButton( 'image', { | |
| icon: 'image', | |
| tooltip: 'Insert Image', | |
| onclick: function() { | |
| var frame = wp.media({ | |
| title: 'Insert Media', | |
| button: { text: 'Insert into HTML Field' }, | |
| multiple: false, | |
| library: { type: 'image' } | |
| } ); | |
| frame.on('select', function() { | |
| var attachment = frame.state().get('selection').first().toJSON(); | |
| editor.insertContent('<img src="' + attachment.url + '" alt="' + attachment.alt + '" />'); | |
| } ); | |
| frame.open(); | |
| } | |
| } ); | |
| // Handle image insertion from media library | |
| editor.addButton( 'image', { | |
| icon: 'image', | |
| tooltip: 'Insert Image', | |
| onclick: function() { | |
| var frame = wp.media({ | |
| title: 'Insert Media', | |
| button: { text: 'Insert into HTML Field' }, | |
| multiple: false, | |
| library: { type: 'image' } | |
| } ); | |
| frame.on('select', function() { | |
| var attachment = frame.state().get('selection').first().toJSON(); | |
| // Escape attributes for security | |
| var url = attachment.url.replace(/"/g, '"'); | |
| var alt = (attachment.alt || '').replace(/"/g, '"'); | |
| // Insert image with more attributes for better accessibility and control | |
| editor.insertContent('<img src="' + url + '" alt="' + alt + '" class="size-full" />'); | |
| } ); | |
| frame.open(); | |
| } | |
| } ); |
There was a problem hiding this comment.
I do dig the suggestion to escape attribute values. The additional class suggestion I'm not sure is needed though.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
@saifsultanc I tested this and it seems to working functionally well!
I did notice a general bug with the snippet though, where upon initial load of the editor, the editor content flashes and then disappears. Including a Loom with that finding since it's easier demonstrated than explain in comment form.
https://www.loom.com/share/c341fcb1ee3e4f5cbea9d32acc3626b1
Again, the changes in the PR are working well, this is just an unrelated bug that I noticed.
| // Handle image insertion from media library | ||
| editor.addButton( 'image', { | ||
| icon: 'image', | ||
| tooltip: 'Insert Image', | ||
| onclick: function() { | ||
| var frame = wp.media({ | ||
| title: 'Insert Media', | ||
| button: { text: 'Insert into HTML Field' }, | ||
| multiple: false, | ||
| library: { type: 'image' } | ||
| } ); | ||
|
|
||
| frame.on('select', function() { | ||
| var attachment = frame.state().get('selection').first().toJSON(); | ||
| editor.insertContent('<img src="' + attachment.url + '" alt="' + attachment.alt + '" />'); | ||
| } ); | ||
|
|
||
| frame.open(); | ||
| } | ||
| } ); |
There was a problem hiding this comment.
I do dig the suggestion to escape attribute values. The additional class suggestion I'm not sure is needed though.
Context
⛑️ Ticket(s): https://secure.helpscout.net/conversation/2898113899/81765
Summary
Adding support for adding images with our Rich Text HTML snippet. Details from David here.
How this update works:
https://www.loom.com/share/dc2214ba5a9c4155899051559b89ce1e