? ? ? Success

User tests: Successful: Unsuccessful:

avatar dgt41
dgt41
13 Jul 2015

As the title suggests: enable drag and drop

This is once again another of those @nikosdion's great ideas, as expressed in #JAB15.

The concept here is very simple: drag an image into the tinyMCE’s content area and the image should automatically be uploaded and then the correct tag should be inserted in the text area.

What’s in it?

There are some minor changes in the com_media file.json.php controller in order to incorporate the needed feedback (namely the relative url for the image) and also to make sure that the name is URL safe (no spaces allowed here).

The tinyMCE plugin introduces a new private function for building the script code required fro the functionality. That’s if the option is enabled in the back end (it is by default).
Also introduced an option for a custom solder (showable only if drag&drop is enabled) to save the images exists
The code also checks if the user is allowed to upload images.
There is a spinning joomla gif to indicate that the upload is actually happening
Also if the image gets successfully uploaded a green overlay will indicate the success
If an error happened the error will display shortly on a red background
For files dropped that are not images nothing will happen
For images that the filename already exists on the server the upload will not happen and the image that is already in the server will appear in the tinyMCE content area. (failsafe we can improve that by introducing incremental filenames as we already do for existing article aliases)

Preview

screenshot 2015-10-25 02 14 10

Testing

Apply the patch
Edit tinyMCE’s options to specify the folder for the images
Test it front end - back end

avatar dgt41 dgt41 - open - 13 Jul 2015
avatar dgt41 dgt41 - change - 13 Jul 2015
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 13 Jul 2015
Labels Added: ?
avatar Bakual
Bakual - comment - 13 Jul 2015

Cool idea :+1:
From the first look I don't like that you put a specific "tiny" controller into com_media. I don't think it is a good idea to have code in com_media which is specific to an editor.
I would try to use the existing file.json controller.

If that doesn't work then I would suggest to use com_ajax and do the upload part as a AJAX plugin event in the editor plugin itself. This way the code for tiny isn't spread all over the place but contained all in the plugin. :smile:
Since Joomla 3.4 or so the plugin doesn't need to be anymore in the ajax group. Thus it works for all plugins as long as the event name (function name) starts with onAjax.... It would also be a great way to showcase what is possible :)

avatar brianteeman
brianteeman - comment - 13 Jul 2015

Didn't the latest version of tiny that you just did a PR for support this
internally?

avatar dgt41
dgt41 - comment - 13 Jul 2015

@Bakual i think I need to talk to @Buddhima and try to get one upload controller that fits all requests. The problem is that new media is not yet production ready!

@brianteeman They introduced a new Upload API but that works with a button next to the field source in the image pop up. try it in their at tiny

avatar brianteeman
brianteeman - comment - 13 Jul 2015

Not just the new upload and seriously cool inline editing here
http://blog.moxiecode.com/2015/06/25/tinymce-4-2-with-image-tools/ but the
release notes (sorry can't find the link right now) talk about drag and drop

avatar mbabker
mbabker - comment - 13 Jul 2015

@brianteeman I think this PR is adding the code needed to turn it on and make the feature work server side (something's gotta process the images).

avatar joomla-cms-bot joomla-cms-bot - change - 13 Jul 2015
Labels Added: ?
avatar Fedik
Fedik - comment - 14 Jul 2015

@dgt41 :+1: very cool! .. here is link for Tiny upload http://www.tinymce.com/wiki.php/Configuration:images_upload_url

I also think that file.upload will be better than tiny.upload ... potentially this can be used by other extensions :wink:

avatar zero-24 zero-24 - change - 14 Jul 2015
Labels Added: ?
avatar zero-24 zero-24 - change - 14 Jul 2015
Easy No Yes
avatar zero-24 zero-24 - change - 14 Jul 2015
Category External Library JavaScript Plugins
avatar dgt41 dgt41 - change - 14 Jul 2015
Title
[New Feature] RFC tinyMCE drag an drop images
[New Feature] RFC tinyMCE drag and drop images
avatar dgt41 dgt41 - change - 14 Jul 2015
Title
[New Feature] RFC tinyMCE drag an drop images
[New Feature] RFC tinyMCE drag and drop images
avatar dgt41 dgt41 - change - 14 Jul 2015
The description was changed
avatar nikosdion
nikosdion - comment - 14 Jul 2015

Where should you put the controller part?

Regarding the controller part, I'd say that it's perfectly acceptable leaving it in com_media. It is generic code which can be reused by other editors or extensions which want to implement asynchronous uploads. It can even be used by com_media itself to implement d'n'd uploads (hint!).

Implementation of the controller

Do you need to call the files array tinyFiles or can we use a more generic name, e.g. uploadedMediaFiles?

Do we really have to go through $_SERVER['CONTENT_LENGTH']? The PHP files arrays do give you the size of uploaded media. Do take a look at the existing helper method JHelperMedia::canUpload. Use it instead of custom code in lines 70:97. Centralised code can be kept secure more easily.

Also, is there a compelling reason for using the code in lines 131:146 instead of JFile::upload? The latter is much better because it allows us to centrally control the security model for uploads. Whenever possible go through the existing helper methods instead of inventing new ways. It's easier for managing the system security.

avatar Bakual
Bakual - comment - 14 Jul 2015

Fully agree with the controller being in com_media when it's generic.
Currently it's named tiny.php which doesn't look generic to me. And we already have a file.json.php controller with an upload task. That's why I suggested to try and use that. It looks like an Ajax upload endpoint, but haven't checked.

avatar nikosdion
nikosdion - comment - 14 Jul 2015

OK, fair enough. Having a single JSON endpoint sounds like a good idea.

avatar dgt41
dgt41 - comment - 14 Jul 2015

@Bakual @nikosdion we still need the controller to return the relative path and the file name of the file uploaded! Which is not available right now, right?

avatar Bakual
Bakual - comment - 14 Jul 2015

I think it should be possible to adjust/extend the existing controller to return that.

You can either add the additional information to the existing JSON response or in worst case add a URL parameter (eg &version=2) which acts as toggle for the response, keeping old behavior for B/C and returning new version of response if parameter is present.

avatar infograf768
infograf768 - comment - 14 Jul 2015

( ! ) Fatal error: Can't use method return value in write context in ROOT/plugins/editors/tinymce/tinymce.php on line 1003

I had patched with 4.2.1 and then this one.

avatar dgt41
dgt41 - comment - 14 Jul 2015

@infograf768 sorry about that JM, latest commit should fix that

avatar infograf768
infograf768 - comment - 14 Jul 2015

hmm. Patched with #7423, then patched with this.
Addedimagetools, in the plugin array line 265 of tinymce.php + changed the line in advanced mode to
plugins : \"table link image code hr charmap autolink lists importcss imagetools\",

Result is negative: I can't see the new icon in the image popup and drag and drop does not work here.

Maybe I missed something?

avatar infograf768
infograf768 - comment - 14 Jul 2015

I also have
Timestamp: 14/07/15 18:37:30
Error: SyntaxError: JSON.parse: unexpected character at line 1 column 1 of the JSON data
Source File: http://localhost:8888/trunkgitnew/administrator/index.php?option=com_content&view=article&layout=edit&id=94
Line: 517

avatar infograf768
infograf768 - comment - 14 Jul 2015

These are also the errors I have when displaying the popup

screen shot 2015-07-14 at 18 40 06

avatar dgt41
dgt41 - comment - 14 Jul 2015

@infograf768 this PR doesn’t require #7423, please try it with the earlier version 4.1.6 (?)

avatar infograf768
infograf768 - comment - 14 Jul 2015

You mean it does not require the new imagetools plugin? I am confused.

avatar dgt41
dgt41 - comment - 14 Jul 2015

drag and drop is not part of the image tools, actually image tools are a bunch of plugins for client side image editing plus some generic upload code. So yes the drag and drop doesn’t really require the images tools

avatar dgt41
dgt41 - comment - 14 Jul 2015

@mbabker thanks Mike. I also made that function static cause it was giving a strict error

avatar mbabker
mbabker - comment - 14 Jul 2015

The class is designed to be all OOP without statics, so change it to this please:

$mediaHelper = new JHelperMedia;

if (!$mediaHelper->canUpload($file, 'com_media'))
...
`
avatar dgt41
dgt41 - comment - 14 Jul 2015

@mbabker Thanks again! Done!

avatar dgt41
dgt41 - comment - 15 Jul 2015

@Bakual Thomas can you review the changes in the controller file.json.php?

avatar Bakual
Bakual - comment - 15 Jul 2015

@dgt41 I don't have time today to do a proper review. What I wonder is if we really need the returnUrl parameter the way you did it. I think you could just always add it to the response without breaking anything. Since the existing status and error response properties remain unchanged. Returning additional data should be backward compatible as long as the existing data isn't changed.
Or do I miss something?

avatar dgt41 dgt41 - change - 16 Jul 2015
The description was changed
Title
[New Feature] RFC tinyMCE drag and drop images
[New Feature] tinyMCE drag and drop images
avatar dgt41 dgt41 - change - 16 Jul 2015
Title
[New Feature] RFC tinyMCE drag and drop images
[New Feature] tinyMCE drag and drop images
avatar dgt41 dgt41 - change - 16 Jul 2015
The description was changed
avatar dgt41
dgt41 - comment - 16 Jul 2015

I think I am done here! Tested and works well on Safari, Firefox, Opera and Chrome. Still haven’t tried on IE but I don’t see any reason why this will fail there (all the code is just some simple jquery stuff).
I would appreciate some feedback form @Bakual @mbabker @nikosdion so if everything is good from code review, people can start testing this

avatar Bakual Bakual - change - 16 Jul 2015
Milestone Added:
avatar Bakual
Bakual - comment - 16 Jul 2015

Controller looks fine now. Thanks for the work you did :+1:
Would be great to get some tests on this.

I've added the 3.5 milestone for now.

avatar peterlose
peterlose - comment - 16 Jul 2015

I can't get it to work in the backend. Nothing happens and no errors. Uploads fine in frontend, however the generated path seems wrong.

skaermbillede 2015-07-16 kl 22 28 51

skaermbillede 2015-07-16 kl 22 29 32

avatar dgt41
dgt41 - comment - 16 Jul 2015

@peterlose I was trying to fix the front end I broke the back end ???? Anyways now should work on both, thanks Peter!
About the path: are you on a subdirectory?
also in tinymce path field you should input something like banners/test relative to root/images

avatar peterlose
peterlose - comment - 16 Jul 2015

@dgt41 Works fine now. Yeah, I'm using a subdirectory. Brain fart.... :)

I'm gonna do some more testing

avatar infograf768
infograf768 - comment - 17 Jul 2015

test negative here, back-end AND front-end, current TinyMCE version is staging/master:

Leaving the field empty for the Image directory and saving the plugin.
When drag and drop an image from any place on my computer "AND the image is not yet in the /images directory", I do not get here the path to the image.
I.e. I get a green screen and then this:
screen shot 2015-07-17 at 10 21 28

Source is:
<p>some text<br /><img src="myimage.png" alt="" /></p>

But the image IS loaded OK in /images

screen shot 2015-07-17 at 10 24 23

If I do it again (in the same article or another one) with the same image dragged from its original location, It is now OK after a brief message:
File already exists
and the image displays fine with source:
<p>some text<br /><img src="images/myimage.png" alt="" /></p>

Firefox, Macintosh.

avatar dgt41
dgt41 - comment - 17 Jul 2015

@infograf768 I was using wrongly the COM_MEDIA_BASE instead of JPATH_ROOT, so all the URLs were relative to /images. Late night coding I guess...

avatar infograf768
infograf768 - comment - 17 Jul 2015

@test
Works now here.
Shall not we test also with #7423 ?

avatar jwaisner
jwaisner - comment - 19 Jul 2015

@test

This PR works as intended and a great feature!


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

avatar jwaisner jwaisner - test_item - 19 Jul 2015 - Tested successfully
avatar jessicadunbar
jessicadunbar - comment - 19 Jul 2015

tested successfully.


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

avatar jessicadunbar jessicadunbar - test_item - 19 Jul 2015 - Tested successfully
avatar Rogitecs
Rogitecs - comment - 19 Jul 2015

I just tested this on Windows 7 and it did not work in Chrome (Version 43.0.2357.134). It did work in Firefox (Version 39) though.

avatar dgt41
dgt41 - comment - 19 Jul 2015

@Rogitecs was there any JavaScript error logged?

avatar Rogitecs
Rogitecs - comment - 19 Jul 2015

@dgt41 I just enabled JavaScript console and retried it again. It did not log any errors. The page is being redirected to file:///C:/Users/Curly/Downloads/rogitecs-logo-joomla-github.jpg as soon as I drag an image.

avatar dgt41
dgt41 - comment - 19 Jul 2015

@Rogitecs I couldn’t replicate it. On my VM win7 chrome Version 43.0.2357.134 everything works well:
img

Did you drop the image inside the content area of tinyMCE? Was the patch still applied?

avatar Rogitecs
Rogitecs - comment - 20 Jul 2015

@dgt41 Hm... Maybe it's related to my browser settings. I tried it one more time, but it didn't work. If someone else can confirm it's working on Windows machine in Chrome, that'll be great. Thanks.

avatar nikosdion
nikosdion - comment - 20 Jul 2015

@dgt41 I have the same problem as @Rogitecs in Google Chrome 43.0.2357.134 (64-bit) under Kubuntu 15.04 (kernel version 3.19.0-22-generic, KDE Plasma 5.2.2). In case it matter I don't use localhost or an IP address as my hostname. I have a custom hosts entry (jalpha.local.web) pointing to my test site.

On the same system this PR works fine with Firefox 39.0. So this issue seems to be isolated to Google Chrome?

How to reproduce (using a fully updated Google Chrome):

  • Install new Joomla! 3.4.3 site
  • Install com_patchtester
  • Search for patch 7435 (this PR) in Joomla! Patch Tester and apply
  • Go to Content, Article Manager, Add New Article
  • Drag and drop an image from the KDE file manager (Dolphin) to the editor area

Expected result: Image being uploaded and inserted in the editor
Actual result: The window URL changes to the dragged file path, replacing the entire page with a display of the image.

avatar brianteeman
brianteeman - comment - 20 Jul 2015

And I thought it was something I was doing wrong

On 20 July 2015 at 08:31, Nicholas K. Dionysopoulos <
notifications@github.com> wrote:

@dgt41 https://github.com/dgt41 I have the same problem as @Rogitecs
https://github.com/Rogitecs in Google Chrome 43.0.2357.134 (64-bit)
under Kubuntu 15.04 (kernel version 3.19.0-22-generic, KDE Plasma 5.2.2).
In case it matter I don't use localhost or an IP address as my hostname. I
have a custom hosts entry (jalpha.local.web) pointing to my test site.

On the same system this PR works fine with Firefox 39.0. So this issue
seems to be isolated to Google Chrome?

How to reproduce (using a fully updated Google Chrome):

  • Install new Joomla! 3.4.3 site
  • Install com_patchtester
  • Search for patch 7435 (this PR) in Joomla! Patch Tester and apply
  • Go to Content, Article Manager, Add New Article
  • Drag and drop an image from the KDE file manager (Dolphin) to the editor area

Expected result: Image being uploaded and inserted in the editor
Actual result: The window URL changes to the dragged file path, replacing
the entire page with a display of the image.


Reply to this email directly or view it on GitHub
#7435 (comment).

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

avatar Fedik
Fedik - comment - 20 Jul 2015

Chrome 43.0.2357.134 (64-bit) Ubuntu 14.04 same problem...
@dgt41 seems Chrome has problem with "on drop" event

avatar Fedik
Fedik - comment - 20 Jul 2015

@dgt41 I just tried add:

ed.on('dragstart', function(e) {
   console.log(ed);return false;
});

and it works in chrome now :smile:
maybe you understand something more from https://gist.github.com/adamayres/3ad2833339079611d18b

seems it what was explained in the answer suggested by @brianteeman :

you must cancel the ondragenter and ondragover events

avatar dgt41
dgt41 - comment - 20 Jul 2015

@nikosdion @brianteeman @Rogitecs can you retest to confirm that the problem is fixed?
I 've also added some visual indication when a file is in the "correct" drop zone, I hope you like it!

avatar brianteeman
brianteeman - comment - 20 Jul 2015

Tested again and it works great.

Also noticed the file already exists message (and in multiple languages - nice) although it did flash a little too quickly on the screen and was hard to see due to the contrast. Did a further test to see what happens if you upload an mage with the same name as one that you have already uploaded. In that case you get the quick message "file already exists" and then see the image that is on the server. This could be confusing. Perhaps the "file already exists" message needs to stay on screen for long or even be something that is dismissable or it should be a "sorry a file with this name already eixsts - pleae rename your file and try again"


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

avatar dgt41
dgt41 - comment - 20 Jul 2015

Thanks Brian for the constructive feedback!
About the file exist and the usage of the file already on the server: I found that returning the path/filename of the existing file might be helpful (in some weirdos mind) but as I pointed in the description what would be optimal in such case is to follow the known path of an existing alias. In such case we need to create a file with a filename that ends in -1and then increment that number for consecutive uploades. that will produce files like:
image.jpg
image-1.jpg
image-2.jpg … etc

But that needs some logic and before I go on and write it, I would like to hear from the PLT if such behavior will be something that will be considered for approval.

About the messages: I guess half a second is not adequate time for any message, but the problem is that on multiple files the time adds up. I guess we need some sort of queue management, I need to think about it.

avatar brianteeman
brianteeman - comment - 20 Jul 2015

Can it be a message you have to dismiss instead of one on a time delay?

It could possibly them have options. Overwrite, rename, dismiss (just thinking out loud)

avatar Rogitecs
Rogitecs - comment - 21 Jul 2015

I just re-tested this and it is fixed in Chrome. When I drag the same image again, I get the message that the file already exists which is fine, but I do agree that it goes away too quickly. I also noticed that even though it detects that the file already exists, it still inserts it.

avatar Bakual
Bakual - comment - 21 Jul 2015

I wouldn't rename the file automatically during upload. Give an error telling the user the file already exists with the name he tried to upload and let him try again.
Overwriting and renaming could cause unwanted sideeffects if done automatically. Prompting the user, would work. However it would probably need much more complex code which is more likely to break, especially with all the various templates out there. Thus I stick with just giving an error message in my own component.

avatar nikosdion
nikosdion - comment - 21 Jul 2015

@test success! VERY BIG thank you, Dimitris :)

Regarding images: yeah, I'd rather get an overlay/popup asking me what to do: overwrite, rename, cancel. However I would be equally fine with just a warning that the file already exists as long as it doesn't go away automatically.


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

avatar nikosdion nikosdion - test_item - 21 Jul 2015 - Tested successfully
avatar dgt41
dgt41 - comment - 29 Jul 2015

@Bakual Thomas should I merge this PR with the other one (xtd buttons)?
This one, as it is right now will create conflicts as soon as the other one gets merged

avatar Bakual
Bakual - comment - 29 Jul 2015

Please don't. They're very different things.
We will just have to deal with the conflicts by then.

avatar zero-24 zero-24 - change - 4 Aug 2015
Status Pending Ready to Commit
avatar zero-24
zero-24 - comment - 4 Aug 2015

RTC based on the lasts tests.


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

avatar joomla-cms-bot joomla-cms-bot - change - 4 Aug 2015
Labels Added: ?
avatar dgt41
dgt41 - comment - 9 Aug 2015

I’ve updated this one more time. Changes:

  • Bring code in line with the other PR for the buttons (I didn’t include that code) to make it easier for commit
  • Moved all the inline script to a relative plugin named 'jdragdrop'
  • Some UI touches: drop zone gets a dashed green border when file is in it
  • Use the native tinyMCE alert box for the error in case that file exists on server

Unfortunately this now needs 2 more tests

avatar peterlose
peterlose - comment - 9 Aug 2015

All good. Works both in frontend and backend

avatar dgt41 dgt41 - change - 9 Aug 2015
Status Ready to Commit Pending
avatar dgt41
dgt41 - comment - 9 Aug 2015

back to pending


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

avatar joomla-cms-bot joomla-cms-bot - change - 9 Aug 2015
Labels Removed: ?
avatar dgt41 dgt41 - alter_testresult - 9 Aug 2015 - jessicadunbar: Not tested
avatar dgt41 dgt41 - alter_testresult - 9 Aug 2015 - jwaisner: Not tested
avatar dgt41 dgt41 - alter_testresult - 9 Aug 2015 - nikosdion: Not tested
avatar dgt41 dgt41 - alter_testresult - 9 Aug 2015 - peterlose: Tested successfully
avatar hdwebpros
hdwebpros - comment - 19 Sep 2015

Tested front end and back end successfully. Also tested trying to upload an image with the same name and successfully get the error. All looks good.


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

avatar hdwebpros hdwebpros - test_item - 19 Sep 2015 - Tested successfully
avatar zero-24
zero-24 - comment - 19 Sep 2015

Thanks @hdwebpros for your test @dgt41 can you sync the staging branch? Than we can do final tests and set RTC ;) Thanks.

avatar dgt41
dgt41 - comment - 21 Sep 2015

@zero-24 the merge conflict was coming from a CS patch in the file.json.php. Now should be ok

avatar zero-24 zero-24 - change - 23 Sep 2015
Status Pending Ready to Commit
avatar zero-24
zero-24 - comment - 23 Sep 2015

Thanks back to RTC :smile:


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

avatar joomla-cms-bot joomla-cms-bot - change - 23 Sep 2015
Labels Added: ?
avatar dgt41
dgt41 - comment - 8 Oct 2015

I was reading the code once again here and catch some stupid bugs:

  • The loader gif had wrong path
  • Added bmp to the logic for checking the file if it is an image
  • The JSON returned message is always named error even if it is a successful upload. Therefore I introduced message, retain the error for the time being but we should definitely deprecate it. It doesn’t make sense.
avatar dgt41 dgt41 - change - 8 Oct 2015
Status Ready to Commit Pending
avatar micker
micker - comment - 9 Oct 2015

hello very interesting feature ! did you add "only" drag and drop or add rezize tool oh 4.2 of tiny ?
regards

avatar joomla-cms-bot joomla-cms-bot - change - 9 Oct 2015
Labels Removed: ?
avatar dgt41
dgt41 - comment - 9 Oct 2015

@micker the image tools of tinyMCE 4.x needs some customization in order to get them to Joomla. In sort we need some kind of ACL for the images, in order to give the right privileges to the author and either enable it or disable the functionality, and also we need to sort out the blob name so we don’t end up with different image names on the server side. The first part is the tricky one, because right now the images don’t have any data stored in the db and therefore is impossible to specify privileges for groups or users. To cut the story sort we need the com_media for this, but it is on my radar ????

avatar dgt41
dgt41 - comment - 9 Oct 2015

@micker One more thing: you can have a plugin that does thumbs if you hook it to the event onContentBeforeSave as there is a triggered event for the images uploaded:

$dispatcher->trigger('onContentBeforeSave', array('com_media.file', &$object_file, true));
avatar micker
micker - comment - 9 Oct 2015

yes the new com_media seems to be stop .... an light thumbs function will be great
thanks team for works !

avatar Andrej1966
Andrej1966 - comment - 24 Oct 2015

In general it works, but I't not really user friendly.
First: There should be any feedback while the upload processes (a process bar)
Second: If you drag a second image while uploading, it breaks (that should be caught)
Third: the "dashed line" looks realy cheap, a designer should look over it.


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

avatar dgt41
dgt41 - comment - 24 Oct 2015

@Andrej1966 About the first and third point: we are not dealing with our own structure here but with tinyMCE. Also you should try tinyMCE’s default behavior on drag and drop. The dashed line is somehow default for drag and drop check Facebook and Twitter, it’s not MY design it’s the default expected.
About trying to drop an image when the spinning logo is present: I guess I should look at this

avatar dgt41
dgt41 - comment - 24 Oct 2015

And here is the styling of Facebook and twitter:

screenshot 2015-10-24 15 47 25
screenshot 2015-10-24 15 49 26

avatar dgt41
dgt41 - comment - 24 Oct 2015

Latest update: I changed the way that the spinner was injected (was with a div overlay, now directly in tinyMCE). This was breaking the functionality if the spinner was shown and another image was dropped in the editor, thanks @Andrej1966 for this.

avatar dgt41
dgt41 - comment - 24 Oct 2015

And a preview of the Bootstrap progress bar for the file upload. Note this will not represent percentage (always 100%) for the moment but it is animated
screenshot 2015-10-25 02 14 10

avatar dgt41
dgt41 - comment - 27 Oct 2015

@Andrej1966 I updated this so now it will have a progress bar

avatar joomla-cms-bot
joomla-cms-bot - comment - 28 Oct 2015

This PR has received new commits.

CC: @hdwebpros, @jessicadunbar, @jwaisner, @nikosdion, @peterlose


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

avatar dgt41 dgt41 - change - 28 Oct 2015
The description was changed
avatar roland-d
roland-d - comment - 28 Oct 2015

Can you guys please test so we can merge it into 3.5? Thanks.

avatar n9iels
n9iels - comment - 31 Oct 2015

Not sure if it is a problem, on my local test installation the image path will be like this:
path
Tested with MAMP, on windows 7.

If I try to upload a .pdf file via drag and drop it gives me the message Unable to upload file.: /joomla-cmsundefined. A message like "File type no supported" might be a better message?

No other problems found, I like it! :smile:

avatar dgt41
dgt41 - comment - 31 Oct 2015

@n9iels about the path: does the image render in tinyMCE?
About the message: this PR was supposed to enhance the existing code of com_media. Obviously we can examine all the messages and maybe put new strings, but the initial idea was that com_media was going to be ready for 3.5.

avatar n9iels
n9iels - comment - 31 Oct 2015

Yes the picture renders correctly and is visible.

I understand, maybe a good idea for a later pull request.
Since the image render correctly I think my test is successfull :)

avatar n9iels n9iels - test_item - 31 Oct 2015 - Tested successfully
avatar n9iels
n9iels - comment - 31 Oct 2015

I have tested this item :white_check_mark: successfully on 5ae9e86


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

avatar joomla-cms-bot
joomla-cms-bot - comment - 1 Nov 2015

This PR has received new commits.

CC: @hdwebpros, @jessicadunbar, @jwaisner, @n9iels, @nikosdion, @peterlose


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

avatar n9iels n9iels - test_item - 1 Nov 2015 - Tested successfully
avatar designbengel
designbengel - comment - 1 Nov 2015

Hi there :)

  • German umlauts are just removed instead of replaced with for example Ö with oe, and Ü with ue, ß with ss, Ä with ae.
  • Successful with / and . inside the Name
  • I named a jpg .png and it was uploaded as .png - i don´t know if there is a possibility to see the real type of a file?
  • Can´t there be a rename dialog if file already exist?
  • by dragging an Illustrator File .ai - the border appears green but nothing happens then. Is it possible to color it red if filetype is not allowed?
  • Is it possible to add image processing (size, cropping,...) on this point, when the dragged picture is way too big?
avatar n9iels
n9iels - comment - 1 Nov 2015

I have tested this item :white_check_mark: successfully on 30dde3e

Slashes are now replaced :+1:


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

avatar dgt41
dgt41 - comment - 1 Nov 2015

Wow that’s a big list ????

  • About the German umlauts: I am using the standard function that is used elsewhere in com_media. If people don’t mind I can mix some transliteration function there. But the idea is either you use this tinyMCE drag and drop or the com_media old upload button you should get the same results. So if I change it for tinyMCE I guess that needs to be updated also for the com_media upload button.
  • About extension rename, it doesn’t really matter as the file metadata is also taken into consideration
    Alt text

  • Rename dialog: This is asynchronous data transfer so it needs some consideration, probably a v2

  • About the red border: Yes it can be easily done IF we are dealing with one file. The problem is that we can have multiple files and this complicates it, so if 2 files are jpg (ok) and one is .ai (not ok) what color should we use? maybe use a blue or grey color like Facebook or twitter?
  • Yes, but it needs to be done through a plugin. But don’t worry I am gonna publish that soon (For free of course). That is the mid term solution, once we get the new com_media we will be able to do so much more…

This was really constructive, thanks @designbengel ????

avatar roland-d
roland-d - comment - 1 Nov 2015

About the German umlauts: I am using the standard function that is used elsewhere in com_media. If people don’t mind I can mix some transliteration function there. But the idea is either you use this tinyMCE drag and drop or the com_media old upload button you should get the same results. So if I change it for tinyMCE I guess that needs to be updated also for the com_media upload button.

If this is the current behaviour I wouldn't change it here and call it out-of-scope and should be dealt with in another PR.

Keep up the good work guys :100:

avatar joomla-cms-bot
joomla-cms-bot - comment - 1 Nov 2015

This PR has received new commits.

CC: @hdwebpros, @jessicadunbar, @jwaisner, @n9iels, @nikosdion, @peterlose


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

avatar dgt41
dgt41 - comment - 1 Nov 2015

Just removed the green color, now the theme color of the toolbar will be used. So default will be light grey (same as twitter dropbox area ???? )

avatar zero-24 zero-24 - test_item - 1 Nov 2015 - Tested successfully
avatar zero-24
zero-24 - comment - 1 Nov 2015

I have tested this item :white_check_mark: successfully on 3bb31c8

works great from a first look.


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

avatar n9iels
n9iels - comment - 1 Nov 2015

Looks better :+1:

avatar designbengel
designbengel - comment - 1 Nov 2015

Thanks for the comments @dgt41 - the grey color is better i think because it does not make the user feel that the system will accept all the images.

There is an error message for duplicate file names, one for too large file size, but not at a wrong file type?

Regarding the umlauts i´d like to open a new issue because thats something really annoying in german when the umlauts are eliminated from the name @roland-d :-)

Can it be set this to change the "paste" position meanwhile the drag and drop process? Now i have to set the marker on the place where the image should be and then drag it anywhere.

Last but not least: it´s funny that after so much years of joomla i just realized that there are plugin parameters for tinymce :-) I´d rather search them in the "Global Configuration" and also expect some Permission Parameter Setting for the Editor - thats surely also another topic...

avatar dgt41
dgt41 - comment - 1 Nov 2015

@designbengel starting from the last comment: there is a PR that will make tinyMCE ACL aware and is planned for 3.5 as well #8147
The paste position in fact is working but there is no blinking cursor and that makes it look like that is not working. Honestly the API of tinyMCE is not very helpful, trying to solve this. It also annoys me, as well!
About the wrong type: there isn’t one because the wrong file types are rejected in the client side and I followed the tinyMCE’s default behavior of silent rejection. I guess we could put another warning modal here…
I think we are getting closer to merging here ????

avatar designbengel
designbengel - comment - 1 Nov 2015

:-) Great - one more suggestion for v2 - Here in github you have a "uploading your files..." text next to the progress icon.

avatar roland-d
roland-d - comment - 2 Nov 2015

@designbengel Can umlauts be used in filenames? They became part of a URL, I don't know if that is a problem. I understand it is an issue and if it can be improved we should but not in this PR though.

Can you share your final test results again? If all is OK, we have 2 successful commits and we can commit this gem.

avatar nikosdion
nikosdion - comment - 2 Nov 2015

@roland-d In theory any UTF-8 character can be used in a URL and supported by all modern browsers and servers. However, if you have any visitors still using IE7 or earlier (don't get me started...) they won't be able to access them. Some servers with questionable localisation settings may also mangle the names. So by necessity we transliterate all URLs to 7-bit ASCII and more specifically in the subset consisting of lowercase unaccented a-z, 0-9, dash, underscore and dot. It's very fun when you have Greek and can transliterate phonetically, by visual resemblance or according to the ISO standard... IMHO we should just silently let people have to use the ASCII subset as we do right now.

@test Still works for me :)

avatar zero-24 zero-24 - alter_testresult - 2 Nov 2015 - nikosdion: Tested successfully
avatar zero-24 zero-24 - change - 2 Nov 2015
Status Pending Ready to Commit
Labels
avatar zero-24
zero-24 - comment - 2 Nov 2015

Thanks lets go for it with 3.5 as first version :+1: -> RTC Thanks @dgt41 this is awesome work


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

avatar joomla-cms-bot joomla-cms-bot - change - 2 Nov 2015
Labels Added: ?
avatar roland-d roland-d - change - 2 Nov 2015
Status Ready to Commit Closed
Closed_Date 0000-00-00 00:00:00 2015-11-02 09:01:34
Closed_By roland-d
avatar roland-d roland-d - reference | 7fae6a0 - 2 Nov 15
avatar roland-d roland-d - merge - 2 Nov 2015
avatar roland-d roland-d - close - 2 Nov 2015
avatar designbengel
designbengel - comment - 2 Nov 2015

@roland-d no i didn´t meant the umlauts to stay in the name like they are. It´s usual to transform ü to ue, ö to oe, ä to ae and ß to ss - currently they are completely removed with is not good :-) so should i open a new request? :-) Fun example: Übergrößenträger -> "Uebergroeßentraeger" or "bergrentrger"

Edit: Oh s* sorry didn´t knew the RTC is removed by a comment - please add it again!

avatar joomla-cms-bot joomla-cms-bot - change - 2 Nov 2015
Labels Removed: ?
avatar roland-d
roland-d - comment - 2 Nov 2015

@designbengel Please create an issue for it, so we don't forget about it :) The RTC has been removed because this feature has been merged.

avatar dgt41 dgt41 - head_ref_deleted - 2 Nov 2015
avatar lungkao
lungkao - comment - 13 Nov 2015

folder for images upload /images only ? if have many picture not good

it possible upload to /images/2015/3/31/id ?

Add a Comment

Login with GitHub to post a comment