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

0.14.43 update causes ckeditor focus to stop reacting in mjml #1649

Closed
kickbk opened this issue Dec 11, 2018 · 7 comments
Closed

0.14.43 update causes ckeditor focus to stop reacting in mjml #1649

kickbk opened this issue Dec 11, 2018 · 7 comments
Labels

Comments

@kickbk
Copy link

kickbk commented Dec 11, 2018

@Art, I realize you stopped supporting your MJML project, but if you don't mind, I noticed an issue that the latest update causes.

To reproduce:

  1. Go to https://grapesjs.com/demo-mjml.html
  2. Double click in the middle of a text element
  3. The text area will highlight and possible one of the words will get selected.
  4. click again anywhere in the middle of the text. For example, try selecting a word.

You will see that it is not possible anymore to select where in the text to position the blinking text cursor or select parts of the text.

This issue is not present in version 0.14.40 and below

@artf
Copy link
Member

artf commented Dec 12, 2018

The same version of grapesjs with the same CKEditor plugin works fine here, so it seems not related to the latest version

@artf artf closed this as completed Dec 12, 2018
@kickbk
Copy link
Author

kickbk commented Dec 13, 2018

Let me demonstrate. I am going to double click a text field and then move around and try to click inside.

This is v0.14.40 - Works
v0 14 40

This is v0.14.43 - Has the issue.
v0 14 43

And same issue on https://grapesjs.com/demo-mjml.html. v0.14.43 - has the issue
grapesjs-mjml

@kickbk
Copy link
Author

kickbk commented Dec 24, 2018

@artf I don't wanna be a pain, but would you please have a look at this issue? It's the same with v0.14.49 and you can see this bug on your demo page https://grapesjs.com/demo-mjml.html

@artf
Copy link
Member

artf commented Dec 27, 2018

mjml demo is not my priority right now, but if you submit a PR I'd be glad to review it

@acamenhas
Copy link

Hi artf, i'm also interesed in this topic too.

Can we have access to the source code of:
https://grapesjs.com/demo-newsletter-editor.html

Thanks

@kickbk
Copy link
Author

kickbk commented Feb 5, 2019

Resolved:
The issue happens with the functions enableEditing and disableEditing in the mjml components.
To fix, replace those functions with the new ones, however, I also update the pointerEvents to none on disable and all on enable.

This can be applied to any component you need to enable and disable editing on.

// place inside gjs-mjml/components/Text.js

...

events: { // We need this to listen to the double click event
	dblclick: 'onActive',
},

onActive(e) {
	// We place this before stopPropagation in case of nested
	// text components will not block the editing (#1394)
	if (this.rteEnabled || !this.model.get('editable')) {
		return;
	}
	e && e.stopPropagation && e.stopPropagation();
	const rte = this.rte;

	if (rte) {
		try {
		this.activeRte = rte.enable(this, this.activeRte);
		} catch (err) {
			console.error(err);
		}
	}

	this.rteEnabled = 1;
	this.toggleEvents(1);

	this.getChildrenContainer().style.pointerEvents = 'all';
},

disableEditing() {
	const model = this.model;
	const editable = model.get('editable');
	const rte = this.rte;
	const contentOpt = { fromDisable: 1 };

	if (rte && editable) {
		try {
			rte.disable(this, this.activeRte);
		} catch (err) {
			console.error(err);
		}

		const content = this.getChildrenContainer().innerHTML;
		const comps = model.get('components');
		comps.length && comps.reset();
		model.set('content', '', contentOpt);

		// If there is a custom RTE the content is just baked staticly
		// inside 'content'
		if (rte.customRte) {
			// Avoid double content by removing its children components
			// and force to trigger change
			model.set('content', content, contentOpt);
		} else { // Not in use since we use a custom rte
			const clean = model => {
				const selectable = !['text', 'default', ''].some(type =>
					model.is(type)
				);
				model.set({
					editable: selectable && model.get('editable'),
					highlightable: 0,
					removable: 0,
					draggable: 0,
					copyable: 0,
					selectable: selectable,
					hoverable: selectable,
					toolbar: ''
				});
				model.get('components').each(model => clean(model));
			};
	
			// Avoid re-render on reset with silent option
			model.trigger('change:content', model, '', contentOpt);
			comps.add(content);
			comps.each(model => clean(model));
			comps.trigger('resetNavigator');
		}
	}

	this.rteEnabled = 0;
	this.toggleEvents();

	this.getChildrenContainer().style.pointerEvents = 'none';
},
...

@lock
Copy link

lock bot commented Feb 7, 2020

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot added the outdated label Feb 7, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Feb 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants