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

TinyMCE drag and drop bug fixes #15057

Merged
merged 8 commits into from
Apr 4, 2017
Merged

TinyMCE drag and drop bug fixes #15057

merged 8 commits into from
Apr 4, 2017

Conversation

dgrammatiko
Copy link
Contributor

@dgrammatiko dgrammatiko commented Apr 2, 2017

Pull Request for Issue ##14906 .

Summary of Changes

Apart from fixing the bug in #14906 this PR also fixes another bug with the toolbar tooltip rendered in wrong position after the first image upload.
Also moves the plugin javascript code out of the tinyMCE/plugins directory (will make updates easier)

Testing Instructions

Edit the configuration and provide a custom path for the drag and drop upload, this setting is per tab option so make sure that you are using the right one!!!
Try to upload an image,
Check where the image is stored and also try to see if the tooltips in bold, italics etc is in the right position

Apply the patch and repeat

Expected result

Use the provided path

Actual result

Is not using the provided path

Documentation Changes Required

None, bug fix

@AlexRed
Copy link
Contributor

AlexRed commented Apr 2, 2017

ok, the patch fix the bug with the editor toolbar tooltip rendered in wrong position after the first image upload.
But for me the "Images Directory" parameter tooltip need to explain you can use the "Images Directory" parameter in the TinyMCE plugin to insert a folder name (not a path). This will be automatically generated by the system on the first dragging of an image. The folder will be created as a subfolder of the one set in the media manager settings as “Path to Files Folder”. Since they are image files, I think it would be better to set that it is created as a subfolder of the one set in the media manager settings as “Path to Images Folder”.

@dgrammatiko
Copy link
Contributor Author

The last commit transforms the inout to a drop down of possible directories, as @brianteeman suggested in the issue.
Personally I always prefer clicking instead of typing (although as a dev it should be the other way around :) )

screen shot 2017-04-02 at 19 57 12

For the tooltip text please make another PR

$options = array();
$root = JComponentHelper::getParams('com_media')->get('image_path');


Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

empty line :-)

*/
class JFormFieldUploaddirs extends JFormFieldList
{
protected $type = 'uploaddirs';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do comment missing

$scriptOptions['uploadUri'] = $uploadUrl;

$externalPlugins = array(
array('jdragdrop' => ($app->isClient('site') ? JUri::root(false) : str_replace('/administrator', JUri::root(false)))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JUri::root() should never contain administrator? :-)

Juri::root() in example.com/administrator returns example.com

@@ -573,7 +573,9 @@ public function onDisplay($name, $content, $width, $height, $col, $row, $buttons

if ($dragdrop && $user->authorise('core.create', 'com_media'))
{
$plugins[] = 'jdragdrop';
$externalPlugins['jdragdrop'] = ($app->isClient('site') ? JUri::root(false) :
str_replace('/administrator', '', JUri::root(false)))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JUri::root() should never contain administrator? :-)

Juri::root() in example.com/administrator returns example.com

@AlexRed
Copy link
Contributor

AlexRed commented Apr 2, 2017

I have tested this item 🔴 unsuccessfully on 330dcc0

no work for me.
I select "headers" in the dropdown and "images/draganddrop" in Path to Files Folder in the Media: Options.
The image go in the new folder images/draganddrop/headers/


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

@AlexRed
Copy link
Contributor

AlexRed commented Apr 2, 2017

Sorry but I can't make a PR for the en language file.. my english is very bad :(


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

@zero-24
Copy link
Contributor

zero-24 commented Apr 2, 2017

Please add a doc block for the new method to fix drone ;)

@dgrammatiko
Copy link
Contributor Author

@AlexRed thanks, that should be ok now

@AlexRed
Copy link
Contributor

AlexRed commented Apr 2, 2017

no more dropdown ?


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

@dgrammatiko
Copy link
Contributor Author

It's still there for me:
screen shot 2017-04-02 at 20 56 33

@AlexRed
Copy link
Contributor

AlexRed commented Apr 2, 2017

sorry, dropdown ok but the image continue to go in a subfolder of Path to Files Folder in the Media: Options.


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

@dgrammatiko
Copy link
Contributor Author

@AlexRed are you on M$ win-dos machine?

@AlexRed
Copy link
Contributor

AlexRed commented Apr 2, 2017

No, linux

@dgrammatiko
Copy link
Contributor Author

@AlexRed so can you post your settings, in tinymce and com_media, I can't really understand why your setup is misbehaving

@dgrammatiko
Copy link
Contributor Author

I select "headers" in the dropdown and "images/draganddrop" in Path to Files Folder in the Media: Options.
The image go in the new folder images/draganddrop/headers/

OK, I've figured why you don't get the expected results.
The tinymce drag and drop folder is a subfolder of the folder that is set in com_media as Images folder.
The system actually behaves correctly, so it uses the "headers" from the tinyMCE options, but the base folder (before that) is from the the com_media. The last part cannot change in 3.x as it will be B/C break so you have to adjust the options in com_media that both folders point in the same directory, something like:
screen shot 2017-04-02 at 21 32 51

@AlexRed
Copy link
Contributor

AlexRed commented Apr 2, 2017

yes, I can. But the Joomla users when select the folder in the dropdown think the images go there... but no. The images go in a new subfolder with the same name if the user use the Path to Files Folder in the Media

@dgrammatiko
Copy link
Contributor Author

But the Joomla users when select the folder in the dropdown think the images go there...

This folder that they will select is always a subfolder of their images directory, maybe a better tooltip there could be more helpful

@AlexRed
Copy link
Contributor

AlexRed commented Apr 2, 2017

yes, also for me the tooltip text need to explain better

@brianteeman
Copy link
Contributor

use the esisting PLG_FIELDS_MEDIA_PARAMS_DIRECTORY_DESC

@joomla-cms-bot joomla-cms-bot added the Language Change This is for Translators label Apr 2, 2017
@dgrammatiko
Copy link
Contributor Author

@brianteeman done

@brianteeman
Copy link
Contributor

I have not tested this item.


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

@dgrammatiko
Copy link
Contributor Author

@brianteeman was that what you wanted to select in the issue tracker?

@brianteeman
Copy link
Contributor

oops no

@brianteeman
Copy link
Contributor

I have tested this item ✅ successfully on 9e6415a


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

@AlexRed
Copy link
Contributor

AlexRed commented Apr 4, 2017

I have tested this item ✅ successfully on 9e6415a

Patch ok for me


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

@ghost
Copy link

ghost commented Apr 4, 2017

RTC after two successful testes.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Apr 4, 2017
@rdeutz rdeutz added this to the Joomla 3.7.0 milestone Apr 4, 2017
@rdeutz rdeutz merged commit a6855d5 into joomla:staging Apr 4, 2017
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Apr 4, 2017
@dgrammatiko dgrammatiko deleted the ++++++++++++++++++++++++++++++++++tinydnd branch July 22, 2017 20:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Language Change This is for Translators
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants