Skip to content

Fix TinyMCE ext-buttons. Respect ext-buttons configuration per an editor instance#14520

Merged
zero-24 merged 7 commits intojoomla:stagingfrom
Fedik:tinymce-hidebuttons
Mar 30, 2017
Merged

Fix TinyMCE ext-buttons. Respect ext-buttons configuration per an editor instance#14520
zero-24 merged 7 commits intojoomla:stagingfrom
Fedik:tinymce-hidebuttons

Conversation

@Fedik
Copy link
Copy Markdown
Member

@Fedik Fedik commented Mar 12, 2017

Pull Request for Issue #14417.

Summary of Changes

This pull restore possibility to hide the ext-buttons per TinyMCE instance.

Testing Instructions

In the article form XML administrator/components/com_content/models/forms/article.xml add one more editor element (somewhere after line <fieldset name="basic" label="COM_CONTENT_ATTRIBS_FIELDSET_LABEL">)

<field name="text2" type="editor" hide="menu,module,contact"
 label="Text2" description="" filter="JComponentHelper::filterText" buttons="true" />

Expected result

Now open the Article editing, you should got:

  • main editor contain all available ex-buttons
  • the additional editor do not have menu,module,contact buttons.

Actual result

Always displayed all buttons.

Documentation Changes Required

nope

@ghost
Copy link
Copy Markdown

ghost commented Mar 13, 2017

I have tested this item 🔴 unsuccessfully on 7efc54b

In none Tab a text2-Editor is shown.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/14520.

@ghost
Copy link
Copy Markdown

ghost commented Mar 13, 2017

I have tested this item ✅ successfully on 7efc54b

On Tab Options Editor Text2 is shown without Menu-, Module-, and Contact-Buttons (first changes in xml-File got not saved).


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/14520.

@Bakual
Copy link
Copy Markdown
Contributor

Bakual commented Mar 29, 2017

@Fedik Can you fix the conflicts in your PR please?

@Bakual
Copy link
Copy Markdown
Contributor

Bakual commented Mar 29, 2017

I have tested this item ✅ successfully on 7efc54b

This fixes the editor buttons for when they are set to hidden in the form xml. It also fixes the issue #14935 for that case.
However both issues remain for the case where the editor is added using a custom editor field.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/14520.

 Conflicts:
	media/editors/tinymce/js/tinymce.min.js
@Fedik
Copy link
Copy Markdown
Member Author

Fedik commented Mar 29, 2017

@Bakual conflict fixed

@Bakual
Copy link
Copy Markdown
Contributor

Bakual commented Mar 29, 2017

Looks like it works now also for the custom fields. Either your conflict solving or me changing some settings in my testing site fixed it.
All good now as far as I see.

@Bakual
Copy link
Copy Markdown
Contributor

Bakual commented Mar 29, 2017

I have tested this item ✅ successfully on 09b6ad3

Tested after conflict solving again.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/14520.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Mar 29, 2017
@jeckodevelopment jeckodevelopment added this to the Joomla 3.7.0 milestone Mar 29, 2017
@laoneo
Copy link
Copy Markdown
Member

laoneo commented Mar 29, 2017

I will test that pr tomorrow morning as well.

@Fedik
Copy link
Copy Markdown
Member Author

Fedik commented Mar 29, 2017

please remove RTC,
later I try to fix #14935 also, it is somehow related

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Mar 29, 2017
@joomla-cms-bot joomla-cms-bot removed this from the Joomla 3.7.0 milestone Mar 29, 2017
@ghost
Copy link
Copy Markdown

ghost commented Mar 29, 2017

Remove RTC as wanted by @Fedik


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/14520.

@PhilETaylor

This comment was marked as abuse.

@Bakual
Copy link
Copy Markdown
Contributor

Bakual commented Mar 30, 2017

There are also merge conflicts now

That's not true:

This branch has no conflicts with the base branch

@PhilETaylor

This comment was marked as abuse.

@Fedik
Copy link
Copy Markdown
Member Author

Fedik commented Mar 30, 2017

There are also merge conflicts now

it is a last problem here 😄

@Fedik
Copy link
Copy Markdown
Member Author

Fedik commented Mar 30, 2017

Please test now again.
+
Test these issues which also should be fixed: #14935 #14990

@Bakual
Copy link
Copy Markdown
Contributor

Bakual commented Mar 30, 2017

I have tested this item ✅ successfully on 96603d2

Tested both with my extension and a custom editor field.

  • Show Button setting is now respected both in form.xml and custom field parameter
  • Hide Button setting is now respected in both places as well and it also works when the parameter is empty
  • Buttons work as expected.
    This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/14520.

@PhilETaylor

This comment was marked as abuse.

@dgrammatiko
Copy link
Copy Markdown
Contributor

I have tested this item ✅ successfully on 96603d2


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/14520.

@dgrammatiko
Copy link
Copy Markdown
Contributor

Thank you @Fedik

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/14520.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Mar 30, 2017
@PhilETaylor

This comment was marked as abuse.

@Bakual Bakual added this to the Joomla 3.7.0 milestone Mar 30, 2017
@PhilETaylor

This comment was marked as abuse.

@zero-24
Copy link
Copy Markdown
Contributor

zero-24 commented Mar 30, 2017

@PhilETaylor i think we can fix the other bug in a new PR. I'm merging this for now and we can fix the other bug with a new PR.

Thanks! 👍

@zero-24 zero-24 merged commit aad4cd2 into joomla:staging Mar 30, 2017
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Mar 30, 2017
@dgrammatiko
Copy link
Copy Markdown
Contributor

@PhilETaylor

This comment was marked as abuse.

@dgrammatiko
Copy link
Copy Markdown
Contributor

dgrammatiko commented Mar 30, 2017

content = (new Function('return ' + options.editor))();

replace it with:

if (window.parent.Joomla && window.parent.Joomla.editors && window.parent.Joomla.editors.instances && window.parent.Joomla.editors.instances.hasOwnProperty(editor)) {
			content = window.parent.Joomla.editors.instances[editor].getContent()
		} else {
			content = (new Function('return ' + options.editor))();
		}

Simple fix

@Fedik Fedik deleted the tinymce-hidebuttons branch March 30, 2017 20:56
@laoneo
Copy link
Copy Markdown
Member

laoneo commented Mar 31, 2017

Good work!

@laoneo
Copy link
Copy Markdown
Member

laoneo commented Mar 31, 2017

Would that mean we can have now different editors on the same page? Then we can think about a setting in the editor custom field to define which editor should be loaded.

@Bakual
Copy link
Copy Markdown
Contributor

Bakual commented Mar 31, 2017

Would that mean we can have now different editors on the same page? Then we can think about a setting in the editor custom field to define which editor should be loaded.

As I understood @DGT41, different editors on the same page work if all editor-xtd plugins are rewritten to the new format. It doesn't work for those using the old format (which will be a lot 3rd party ones). So for now I wouldn't add the option to specify the editor type. With 4.0 we can add it.

@dgrammatiko
Copy link
Copy Markdown
Contributor

Would that mean we can have now different editors on the same page?

Not so soon :(
The problem is that all the existing editors (apart the core ones) are still using the old jInsertEditorText() which by design is not supporting multiple editors (different type).
On top of that as @Bakual wrote above all the XTDs are using as well the old method jInsertEditorText().

The only thing we could do is prepare all the plugins to use the new API if the editor supports it e.g.

if (window.parent.Joomla && window.parent.Joomla.editors && window.parent.Joomla.editors.instances && window.parent.Joomla.editors.instances.hasOwnProperty(editor)) {
	content = window.parent.Joomla.editors.instances[editor].getContent()
} else {
	content = (new Function('return ' + options.editor))();
}

Also there is another instance that needs to follow the API in /administarator/components/com_fields/view/fields/tmpl/modal.php
instead of

fieldIns = function(id) {
	window.parent.jInsertEditorText("{field " + id + "}", "' . $editor . '");
	window.parent.jModalClose();
};
fieldgroupIns = function(id) {
	window.parent.jInsertEditorText("{fieldgroup " + id + "}", "' . $editor . '");
	window.parent.jModalClose();
};

we should use

fieldIns = function(id) {
if (window.parent.Joomla && window.parent.Joomla.editors && window.parent.Joomla.editors.instances && window.parent.Joomla.editors.instances.hasOwnProperty(editor)) {
	window.parent.Joomla.editors.instances[editor].replaceSelection("{field " + id + "}")
} else {
	window.parent.jInsertEditorText("{field " + id + "}", "' . $editor . '");
}
	window.parent.jModalClose();
};
fieldgroupIns = function(id) {
if (window.parent.Joomla && window.parent.Joomla.editors && window.parent.Joomla.editors.instances && window.parent.Joomla.editors.instances.hasOwnProperty(editor)) {
        window.parent.Joomla.editors.instances[editor].replaceSelection("{fieldgroup " + id + "}")
} else {
	window.parent.jInsertEditorText("{fieldgroup " + id + "}", "' . $editor . '");
}
	window.parent.jModalClose();
};

@PhilETaylor

This comment was marked as abuse.

@dgrammatiko
Copy link
Copy Markdown
Contributor

@PhilETaylor if we apply these changes to read more and fields modal view then all core components are supporting multiple (different types) editors per page.
But all 3rd PD editors and XTDs will not if they don't update their code to use the API

@PhilETaylor

This comment was marked as abuse.

@dgrammatiko
Copy link
Copy Markdown
Contributor

ok then, someone needs to copy paste those code snippets and create the PRs

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.

8 participants