gw-rich-text-html-fields.php: Added functionality to remember the last selected Visual or HTML editor mode.
#1071
Conversation
WalkthroughThis change updates the editor functionality in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant EditorScript
participant FieldPropertyStore
participant EditorInstance
User->>EditorScript: Load form field settings
EditorScript->>FieldPropertyStore: Retrieve saved editor mode (gwRichTextMode)
EditorScript->>EditorInstance: waitForEditorToBeReady()
EditorInstance-->>EditorScript: Editor ready
EditorScript->>EditorInstance: Switch editor to saved mode (tmce or html)
sequenceDiagram
participant User
participant Editor
participant EventListener
participant SetFieldContentProperty
participant FieldPropertyStore
User->>Editor: Input or switch editor mode
Editor->>EventListener: Detect input or mode change
EventListener->>SetFieldContentProperty: Trigger content update
alt HTML mode active
SetFieldContentProperty->>Editor: Get plain text content
else TinyMCE mode active
SetFieldContentProperty->>Editor: Get TinyMCE content
end
SetFieldContentProperty->>FieldPropertyStore: Save current editor mode (gwRichTextMode)
Possibly related PRs
Suggested reviewers
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 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 (
|
|
@malayladu Not a review, just a hot thought – my instinct would be to save this to the field in the form meta. This way if they open the field in another browser they'll still have the same preference set. |
malayladu
force-pushed
the
malay/fix/81917-content-save-issue
branch
2 times, most recently
from
0af90a0 to
7541990
Compare
![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
🧹 Nitpick comments (5)
gravity-forms/gw-rich-text-html-fields.php (5)
75-77: Good implementation of editor mode persistenceStoring the previously selected editor mode in localStorage is a good approach for persisting user preferences across page reloads.
Consider implementing server-side persistence as mentioned in the PR comments by spivurno - saving this preference to the form meta would ensure consistency across different browsers/devices.
78-86: Add timeout mechanism to the editor readiness checkThe polling approach to check if the TinyMCE editor is initialized is good, but could be improved.
Add a timeout mechanism to prevent potential infinite polling:
function waitForEditorToBeReady(callback) { + var maxAttempts = 50; // 5 seconds max wait time + var attempts = 0; var interval = setInterval(function () { + attempts++; if (typeof tinymce !== 'undefined' && tinymce.get(editorId)) { clearInterval(interval); callback(); } + else if (attempts >= maxAttempts) { + clearInterval(interval); + console.warn('Editor initialization timed out'); + } }, 100); }
88-94: Be explicit about the default mode conditionThe code switches to the saved mode or defaults to Visual mode if the saved mode isn't 'html'.
Make the intent clearer by explicitly checking for both possible values:
waitForEditorToBeReady(function () { if (savedMode === 'html') { window.switchEditors.go(editorId, 'html'); - } else { + } else { // Default to visual mode if savedMode is null, undefined, or 'tmce' window.switchEditors.go(editorId, 'tmce'); } });
96-108: Consider namespacing the global functionCreating a function to update content based on the current mode is a good approach.
However, exposing the function directly on the
windowobject could lead to naming conflicts. Consider namespacing it:- window.SetFieldContentProperty = function () { + window.GravityWizRichText = window.GravityWizRichText || {}; + window.GravityWizRichText.setFieldContentProperty = function () { var mode = jQuery('#wp-' + editorId + '-wrap').hasClass('html-active') ? 'html' : 'tmce'; var content = ''; if (mode === 'html') { content = jQuery('#' + editorId).val(); } else if (tinymce.get(editorId)) { content = tinymce.get(editorId).getContent(); } SetFieldProperty('content', content); };Then update all references to this function accordingly.
110-113: Optimize event binding and ensure it handles all input casesThe current implementation binds to the change event, which may not capture all user interactions.
Consider using more specific event binding and adding additional events to catch all user inputs:
// Update the content. - jQuery(document).on('change', `#${editorId}`, function () { - window.SetFieldContentProperty(); + jQuery(`#${editorId}`).on('change input keyup', function () { + window.SetFieldContentProperty(); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
gravity-forms/gw-rich-text-html-fields.php(1 hunks)
🔇 Additional comments (1)
gravity-forms/gw-rich-text-html-fields.php (1)
121-122: Good implementation of mode persistenceSaving the current mode to localStorage when switching editors ensures the preference is maintained.
Consider testing this implementation across different browsers to ensure consistent behavior, particularly older browsers that may have limited localStorage support.
@spivurno That’s a great idea for keeping the choice persistent across devices and browsers. However, to make that work, we’d also need to save user-specific preferences for the form. If multiple people are working on the same form, we’ll need to store preferences individually for each user. Do we want to implement this? |
You are right. Since form meta applies globally, we'd need to introduce a way to store these preferences on a per user basis. We could potentially explore storing the preference using user meta key to the form ID? |
malayladu
force-pushed
the
malay/fix/81917-content-save-issue
branch
from
7541990 to
7547d70
Compare
|
@malayladu @saifsultanc @claygriffiths I think I understand the confusion on my original feedback that this should be saved to the form meta. By default, WordPress' TinyMCE implementation does treat visual vs text editing as a user-specific preference. And it makes sense that we might want to preserve that. My concern is that toggling between these views can actually break custom markup as the visual editor will transform the text-based content into its own desired format. This makes me want to break with the norm here and make the format (visual vs text) a field-specific setting rather than a user-specific preference. Thoughts? EDIT: Just wanted to add that this snippet is proving to be quite popular and gaining a lot of interest so I think it's worth getting this right. If we need to punt it in favor of more pressing tickets that's totally fine – but hopefully not for too long. 😄 |
@spivurno With the latest code update, the editor mode setting is now saved separately for each field |
|
@malayladu What I'm saying is that local storage doesn't solve the problem we're trying to solve here. Local storage is inherently a user preference. Many different users will edit a given form and interact with these fields. If this is a user preference than the carefully crafted markup from User A could be unintentionally undone by User B just by loading the field settings for a Rich Text HTML field. Some examples from the wild:
I don't think we'll need any fancy code to pull this off. Here's what I'm thinking.
In this scenario, I don't think we need local storage at all. Thoughts? |
Ah, I see now. We want the editor mode setting to be based on the field, not the user. So once it's saved, any user who loads that field will see it in the |
|
@malayladu Exactly! 😄 |
…ast selected `Visual` or `HTML` editor mode.
malayladu
force-pushed
the
malay/fix/81917-content-save-issue
branch
from
7547d70 to
814c201
Compare
Context
⛑️ Ticket(s): https://secure.helpscout.net/conversation/2901552081/81917
Summary
This PR fixes this two issues.