? Failure
Related to # 15059

User tests: Successful: Unsuccessful:

avatar adamasantares
adamasantares
6 Aug 2016

Summary of Changes

TinyMce always ignored custom size for editor and it's a problem for developers who use editors in his components and they can't change the height of this editor.

avatar joomla-cms-bot joomla-cms-bot - change - 6 Aug 2016
Category Plugins Front End
avatar adamasantares adamasantares - open - 6 Aug 2016
avatar adamasantares adamasantares - change - 6 Aug 2016
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 6 Aug 2016
Labels Added: ?
avatar adamasantares adamasantares - change - 6 Aug 2016
The description was changed
avatar adamasantares adamasantares - edited - 6 Aug 2016
avatar brianteeman
brianteeman - comment - 6 Aug 2016

Can you fix the code style issues please

FOUND 2 ERROR(S) AFFECTING 2 LINE(S)

330 | ERROR | Expected "if (...)\n...{...}\n...else\n"; found "if (...)
| | ...{...}\n...else\n"
334 | ERROR | Expected "if (...)\n...{...}\n...else\n"; found "if (...)

| | ...{...}\n...else\n"

avatar infograf768
infograf768 - comment - 6 Aug 2016

hmm, there are cs errors indeed, but Travis gets that weird.

Looks it is just a matter of moving the { to next line and afew others, see below :)

avatar adamasantares
adamasantares - comment - 6 Aug 2016

updated, now it's correct?

avatar brianteeman
brianteeman - comment - 6 Aug 2016

Now it has passed the unit tests and needs some real humans to test it

avatar adamasantares
adamasantares - comment - 6 Aug 2016

ok, thanks

avatar brianteeman
brianteeman - comment - 7 Aug 2016

Can you provide some test instructions please. I tried a few things but didnt see any changes


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

avatar adamasantares
adamasantares - comment - 8 Aug 2016

well, I check it and found that this fix work only for JFormFieldEditor class (when used form.xml). But it doesnt work for "JEditor" class. So need another fix also.

avatar adamasantares
adamasantares - comment - 8 Aug 2016

Ups, I was wrong, it works fine too.

Use this code in some "view"

        $editorType = JFactory::getConfig()->get('editor');
        $editor = JEditor::getInstance($editorType);
        $editor->initialise();
        echo $editor->display('content_text', 'some text here', '400', '150', '60', '20'); // width: 400, height: 150
avatar brianteeman brianteeman - test_item - 8 Aug 2016 - Tested unsuccessfully
avatar brianteeman
brianteeman - comment - 8 Aug 2016

I have tested this item 🔴 unsuccessfully on 8197497

Following your test instructions I found that the height did change but the width did not


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

avatar ggppdk
ggppdk - comment - 8 Aug 2016

@brianteeman

Following your test instructions I found that the height did change but the width did not

i confirm same behaviour

  • but this is as good as we can get it without hacking tinymce.min.js ??

Here is some code from tinymce.min.js, you see that:
width is hard-coded to 100% and height is parametric ?? !!

... Press ALT-0 for help"),
style:{width:"100%",height:o,display:"block"}

Is it a bug, is it deliberate break of tinymce configuration ? i don't know, maybe i am missing something

To see the effect
e.g. at contact form has an editor field

https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_contact/models/forms/contact.xml#L89-L96

add: height="217" width="854"

you will see that height works at editor
and both width and height work if you click "toggle editor"

we could add an div container external to the iframe and set width to it ??
but that will cause the width to behave as max-width, thus then you will not be able to enlarge the editor, any other ideas ?

avatar ggppdk
ggppdk - comment - 8 Aug 2016

And that code inside tinymce.min.js is 1 place,
similar hard-coding must be in more than 1 places ?

avatar dgt41
dgt41 - comment - 8 Aug 2016

Guys, tinymce is rendered as a field, so being 100% from the script is ok, as we still have control over the parent div. in simple words the width should be applied in the jlayout at tinymce's parent div.

EDIT: just edit here:
https://github.com/joomla/joomla-cms/blob/staging/plugins/editors/tinymce/tinymce.php#L985

        $width  = ($textarea->width) ? ' style="width: ' . (int) $textarea->width . 'px"' : '';
        $editor = '<div class="editor"' . $width . '>';

This should fix that:

screen shot 2016-08-08 at 20 45 18

avatar adamasantares
adamasantares - comment - 9 Aug 2016

@dgt41 I think you are right. Will do that.

PS: this is not super solution, but I don't think that update of tinymce source is better.

PPS: how about of new editor? http://mindmup.github.io/bootstrap-wysiwyg/ looks nice.

avatar ggppdk
ggppdk - comment - 9 Aug 2016

@dgt41

very nice try, but have you seen my comment above ?

we could add an div container external to the iframe and set width to it ??
but that will cause the width to behave as max-width, thus then you will not be able to enlarge the editor, any other ideas ?

so what you suggest is similar to the above,
do we want it acting as a max-width , thus prevent enlarging ?

  • i think we can only do it by adding width via JS that runs on-load-complete event of the editor, and adds width at 3 boxes that need it
avatar ggppdk
ggppdk - comment - 9 Aug 2016

http://archive.tinymce.com/wiki.php/API3:event.tinymce.Editor.onPostRender
(which is for tinymce 3, what is the page for tinymce 4 ?)

We just need to find the 2-4 places that are resized when editor resize and add an initial width
like:
$width
$width-15

avatar dgt41
dgt41 - comment - 9 Aug 2016

@ggppdk the editor CAN NOT be resized more than the parent div. This will patch also another bug #8886 read my comment there: #8886 (comment)

avatar ggppdk
ggppdk - comment - 9 Aug 2016

Yes exactly, we agree on it

that is why i said not to add it to at: <div class="editor" ...
it does not only act as max-width,

  • the display also gets cropped if you try to enlarge it
avatar ggppdk
ggppdk - comment - 9 Aug 2016

@dgt41

so i do not mean the container external to the editor
i mean the containers inside the tinymce code that are created after the editor renders

avatar ggppdk
ggppdk - comment - 9 Aug 2016

I have made 2 PRs to your branch (undoing last commit and doing the following)

This code works
Replace:

        tinymce.init({
        ";

with:

        function tinymce_setInitialWidth_".$id."(editor)
        {
            jQuery('iframe#".$id."_ifr').closest('.editor').find('.mce-container.mce-panel')
                .first().css({ 'width': '".$width."' });
            jQuery('iframe#".$id."_ifr').css({ 'width': '100%' });
        }

        tinymce.init({
            init_instance_callback : tinymce_setInitialWidth_".$id.",
        ";

[EDIT]
works when using percentage too

avatar dgt41
dgt41 - comment - 9 Aug 2016

@ggppdk tinyMCE is vanilla js, blending jQuery is wrong IMHO (I know I done it elsewhere and I need to undo those instances)

avatar ggppdk
ggppdk - comment - 9 Aug 2016

@ggppdk tinyMCE is vanilla js, blending jQuery is wrong IMHO (I know I done it elsewhere and I need to undo those instances)

yes and i was thinking of it !!, while i was adding it
and you are right about it
it is best to keep it indepedent of jQuery,

So i did a text search in the file and found jQuery being used already , so i said i will use it

  • when you replace the other jQuery selectors, can you replace these selectors too ?
avatar brianteeman
brianteeman - comment - 9 Aug 2016

lets not make it worse than it is and not create NEW jquery dependent stuff

On 9 August 2016 at 11:18, Georgios Papadakis notifications@github.com
wrote:

@ggppdk https://github.com/ggppdk tinyMCE is vanilla js, blending
jQuery is wrong IMHO (I know I done it elsewhere and I need to undo those
instances)

yes and i was thinking of it !!, while i was adding it
and you are right about it
it is best to keep it indepedent of jQuery,

So i did a text search in the file and found jQuery being used already
, so i said i will use it

  • when you replace the other jQuery selectors you will replace these selectors too ?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#11489 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABPH8Wl97ovOGoqiiINVDAv4D_yRVzM1ks5qeFQIgaJpZM4JeMwb
.

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

avatar ggppdk
ggppdk - comment - 9 Aug 2016

lets not make it worse than it is and not create NEW jquery dependent stuff

no problem, i will replace the selectors and methods with native JS

avatar adamasantares
adamasantares - comment - 11 Aug 2016

wow, I missed so much. Should I do something or my help no needed?

avatar wojsmol
wojsmol - comment - 11 Aug 2016
avatar adamasantares
adamasantares - comment - 11 Aug 2016

@wojsmol umm, sorry, may be I'm inattentive, but...what is the difference? That lines looks same... no?

UPD: ok, I think I see now. You are changed spaces to tabs

avatar adamasantares
adamasantares - comment - 12 Aug 2016

@brianteeman I propose to merge the current realization.

avatar franz-wohlkoenig franz-wohlkoenig - change - 3 Apr 2017
Rel_Number 0 15059
Relation Type Related to
avatar joomla-cms-bot joomla-cms-bot - edited - 3 Apr 2017
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 25 Aug 2018

If this Issue get no Response, it will be closed at 22th September 2018.

avatar adamasantares
adamasantares - comment - 27 Aug 2018

@franz-wohlkoenig sorry I have no time to continue with this issue...

avatar adamasantares adamasantares - change - 27 Aug 2018
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2018-08-27 05:50:29
Closed_By adamasantares
avatar adamasantares adamasantares - close - 27 Aug 2018
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 27 Aug 2018

@adamasantares thanks for your Work and Time.

Add a Comment

Login with GitHub to post a comment