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

[4.1] TinyMCE should prefer the editor dimensions and fallback to the plugin params #37603

Merged
merged 7 commits into from May 17, 2022

Conversation

laoneo
Copy link
Member

@laoneo laoneo commented Apr 22, 2022

Pull Request for #9812, #8591, #5820, #11851 and #8106.

Summary of Changes

The TinyMCE editor forces the plugin width and height instead of using the dimensions from the editor itself. This editor fixes it.

Testing Instructions

Test 1:

  • Set a width value of 200 and height of 300 in the TinyMCE plugin in the advanced tab and save.
  • Add the following code to the file administrator/components/com_content/tmpl/articles/default.php somewhere on the top
    echo JEditor::getInstance('tinymce')->display('description', '', '', '', '60', '5', false);
  • Open the url /administrator/index.php?option=com_content&view=articles
  • Inspect the editor with the dev tools in the browser.

Test 2:

  • Add the following code to the file administrator/components/com_content/tmpl/articles/default.php somewhere on the top
    echo JEditor::getInstance('tinymce')->display('description', '', '450', '400', '60', '5', false);
  • Open the startpage of your Joomla installation
  • Inspect the editor with the dev tools in the browser.

Test 3:

  • In the file administrator/components/com_content/forms/article.xml replace line 54 with
    buttons="true" height="370" width="380"
  • Open the article form
  • Inspect the editor with the dev tools in the browser.

Actual result BEFORE applying this Pull Request

Test 1:
Dimensions are 200x300.

Test 2:
Dimensions are 200x300.

Test 3:
Dimensions are 200x300.

Expected result AFTER applying this Pull Request

Test 1:
Dimensions are 200x300.

Test 2:
Dimensions are 450x400.

Test 3:
Dimensions are 370x380.

@richard67 richard67 added the PBF Pizza, Bugs and Fun label Apr 22, 2022
@richard67
Copy link
Member

richard67 commented Apr 22, 2022

@laoneo I am a bit confused by the title of this PR, which starts with "[4.2]" while target branch is 4.1-dev. What is right?

@brianteeman
Copy link
Contributor

brianteeman commented Apr 22, 2022

There is no point in this pr as it's changed in tinymce 6

sorry this bit stays the same I misread the hcanges and the new docs were not published at the time

@laoneo laoneo changed the title [4.2] TinyMCE should prefer the editor dimensions and fallback to the plugin params [4.1] TinyMCE should prefer the editor dimensions and fallback to the plugin params Apr 22, 2022
@laoneo
Copy link
Member Author

laoneo commented Apr 22, 2022

There is no point in this pr as it's changed in tinymce 6

What has changed? Do you see the problem this pr addresses?

@brianteeman
Copy link
Contributor

I am aware of the problem this addresses and I am trying to save you time as I am also aware of the changes in tinymce 6 and that at least one of the changes is directly related to the editor dimensions

@laoneo
Copy link
Member Author

laoneo commented Apr 22, 2022

Do you have a link to the change?

@richard67
Copy link
Member

New issue in the list of issues fixed by this PR see #37723 .

@Fedik
Copy link
Member

Fedik commented May 2, 2022

Sorry, this not going to work :

* @param string $width The width of the text area (px or %).
* @param string $height The height of the text area (px or %).

Note: (px or %)

Not that easy, and that probably a reason why this bug live so long ;)

We can probably drop % part, if it will work safely

@brianteeman
Copy link
Contributor

https://www.tiny.cloud/docs/configure/editor-appearance/#height

If it is a number then tinymce assumes it is px
If it is a string then tinymce uses the units

@brianteeman
Copy link
Contributor

omg we've all missed the obvious new pr incoming

@richard67
Copy link
Member

omg we've all missed the obvious new pr incoming

Which one?

@brianteeman
Copy link
Contributor

@Fedik actually this works fine with both % and px. We do not need to have any of the lines with the is_numeric check as tinymce automatically sees a number as px without us adding anything

html_height = is_numeric($html_height) ? $html_height . 'px' : $html_height;

so all that is needed is to change

		$html_height = $this->params->get('html_height', '550');
		$html_width  = $this->params->get('html_width', '');
		$html_width  = $html_width == 750 ? '' : $html_width;
		$html_width  = is_numeric($html_width) ? $html_width . 'px' : $html_width;

to

		$html_width  = $width ?: $this->params->get('html_width', '');
		$html_height = $height ?: $this->params->get('html_height', '550');

and we can also delete lines 68 and 69 as they are not needed

@laoneo
Copy link
Member Author

laoneo commented May 2, 2022

I did that in this pr, or is there something missing?

@brianteeman
Copy link
Contributor

@laoneo you just need to delete the 4 lines with isnumeric as they are not needed

@laoneo
Copy link
Member Author

laoneo commented May 9, 2022

@Fedik thanks for the hint, added the check in 95fd92c.

@JonasAtZwetschke
Copy link

I have tested this item ✅ successfully on 177c2ef


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

1 similar comment
@Fedik
Copy link
Member

Fedik commented May 14, 2022

I have tested this item ✅ successfully on 177c2ef


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

@Fedik
Copy link
Member

Fedik commented May 14, 2022

r2c


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label May 14, 2022
@bembelimen bembelimen merged commit e0b4ff8 into joomla:4.1-dev May 17, 2022
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label May 17, 2022
@bembelimen
Copy link
Contributor

Thx

@bembelimen bembelimen added this to the Joomla 4.1.4 milestone May 17, 2022
@laoneo laoneo deleted the j4/mce/dimensions branch May 17, 2022 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PBF Pizza, Bugs and Fun
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants