Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Close SourceEditing view on plugin disabling #17729

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Close SourceEditing view on plugin disabling #17729

wants to merge 4 commits into from

Conversation

l-lejman
Copy link
Contributor

@l-lejman l-lejman commented Jan 9, 2025

Suggested merge commit message (convention)

Other (source-editing): The SourceEditing plugin now closes its view upon being disabled, preventing users from remaining stuck in this mode.


Additional information

A change is needed to ensure that SourceEditing is properly closed when CommentsOnly mode is enabled or when SourceEditing is disabled by API calls. These two plugins should not operate simultaneously.

@f1ames
Copy link
Contributor

f1ames commented Jan 10, 2025

It generally works as intended, but there is more general topic to discuss. What happens with any modifications while the source mode gets disabled programmatically? The current implementation is done in a way that all changes are automatically propagated (I also reported a related issue which somehow touches it - #17738).

This is because when isSourceEditingMode flag changed (which basically turns source mode on/off in every case), the editor data is updated:

this.on( 'change:isSourceEditingMode', ( evt, name, isSourceEditingMode ) => {
if ( isSourceEditingMode ) {
this._hideVisibleDialog();
this._showSourceEditing();
this._disableCommands();
} else {
this._hideSourceEditing();
this._enableCommands();
}
} );

private _hideSourceEditing(): void {
const editor = this.editor;
const editingView = editor.editing.view;
this.updateEditorData();

I'll bring this topic to broader discussion so we can decide how to move forward. Maybe that was one of the reasons it earlier applied read-only mode in similar scenarios instead of switching back to WYSIWYG mode?

@scofalik
Copy link
Contributor

Sorry for the confusion, as I suggested that maybe source editing should close if the plugin is disabled, but I didn't fully understand how this plugin operates now and what we want to achieve.

Right now, the plugin has some issues, both from:

  • product perspective (e.g. if read-only mode kicks in when you are in source mode, you are stuck in it, also it should be possible to see the source in read-only mode, just not change it), and
  • from code perspective (e.g. the plugin does not have any commands and the logic is written in unfortunate way, e.g. closing source mode is coupled with saving the data).

We won't solve all the problems without a bigger refactor, so let's stick to something that makes sense and is quick.

Right now, when you switch to read-only following things happen in source editing plugin:

  1. Toolbar and menu bar buttons for source mode are disabled. You can't switch to/from the source mode.
  2. Textarea is disabled.

Let's go with exactly same behavior for when the plugin is disabled (e.g. for comments-only mode, note, that comments-only mode will be set to true or false at the beginning of the editor initialization and likely won't change after).

So, if the plugin gets disabled (comments-only is activated), we will disable the buttons and textareas.

And I think that for this behavior we don't need any change in the plugin, correct?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants