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

Add drag & drop installation #14905

Closed
wants to merge 13 commits into from

Conversation

asika32764
Copy link
Contributor

@asika32764 asika32764 commented Mar 26, 2017

Summary of Changes

Add drag & drop file to install extension package.

The drag zone is whole page of the Extensions -> Manage -> Install view and it works for 4 tabs now only works for Upload package file tab.

Testing Instructions

Go to Extensions -> Manage -> Install and drag file from desktop to browser. Joomla will auto upload it to install.

Images

drag-upload

@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators PR-staging labels Mar 26, 2017
@dgrammatiko
Copy link
Contributor

Formdata is not supported in IE8 http://caniuse.com/#search=formdata
so we need some sort of fallback (in tinymce we have a message)

@brianteeman
Copy link
Contributor

First of all thank you very much for doing this. I drag and drop all the time to upload extensions and its a pain to hit the very small target so this will make a great difference

We have drag and drop in one other place in Joomla which is for images within tinmyce and it would be good if you could make the language string here match.

So can you change the string to "Drag and Drop ...." not just Drag

Secondly I can see in the tinymce plugin for dragndrop (media\editors\tinymce\plugins\jdragdrop\plugin.js) that we have a check and an error if the users browser doesnt support drag drop. So I presume we need a similar to check here?

@brianteeman
Copy link
Contributor

I see that @dgt41 was writing the same thing at the same time as me ;)

@asika32764
Copy link
Contributor Author

asika32764 commented Mar 26, 2017

Thanks for the feedback, I'll try fix the IE8 and language issues later.

@asika32764
Copy link
Contributor Author

asika32764 commented Mar 26, 2017

Added an alert to notice IE8 users which is not support drag&drop upload. I don't have Windows now, could anyone help me to test?

The language key wrote by @Dg41 is only for Tinymce plugin and hard coded the image words in language string, so I added 2 new language keys for this feature in com_installer.

@infograf768
Copy link
Member

It works, but one has no idea that Drag and Drop is possible at looking at the UI.
I suggest adding this information on the page.

@brianteeman
Copy link
Contributor

brianteeman commented Mar 26, 2017 via email

@@ -12,6 +12,8 @@ COM_INSTALLER_CONFIRM_UNINSTALL="Are you sure you want to uninstall? Confirming
COM_INSTALLER_CURRENT_VERSION="Installed"
COM_INSTALLER_DISCOVER_FILTER_SEARCH_DESC="Search in discovered extension name. Prefix with ID: to search for an extension ID."
COM_INSTALLER_DISCOVER_FILTER_SEARCH_LABEL="Search Discovered Extensions"
COM_INSTALLER_DRAG_ERR_UNSUPPORTEDBROWSER="Drag and drop file upload is not available for your browser. Please consider using a fully HTML5 compatible browser"
Copy link
Contributor

Choose a reason for hiding this comment

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

perfect text - please add a . at the end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also fixed the original text from TinyMCE

@asika32764
Copy link
Contributor Author

It works, but one has no idea that Drag and Drop is possible at looking at the UI.
I suggest adding this information on the page.

I have considered about this but I didn't implement this message yet, I'm open to let people decide what to do.

@ghost
Copy link

ghost commented Mar 26, 2017

@infograf768 i guess most Users are surprised that its not working (like this PR allow). So you have experienced Users and they should got Info by Release-Data.

@infograf768
Copy link
Member

When we edit TinyMCE plugin, it is indicated.
screen shot 2017-03-26 at 12 29 09

It is indeed not indicated when using tinyMCE as an editor to drop images into the textarea.

@ghost
Copy link

ghost commented Mar 26, 2017

I have tested this item ✅ successfully on 9641792


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

@brianteeman
Copy link
Contributor

@infograf768 that is for the drag and drop of moving the tinymce buttons. NOT for the drag and drop image uploading ;)

@brianteeman
Copy link
Contributor

@franz-wohlkoenig it has always worked (a bit) - just that you had to drop it on the button and then click on the upload button. This PR makes it work as it should

@brianteeman
Copy link
Contributor

would something like this be acceptable to you

screenshotr11-39-30

@jeckodevelopment
Copy link
Member

In many applications, when there's the opportunity to drag and drop, there's a kind of dashed border to highlight the area in which you can drag and drop the file/whatever

@Bakual
Copy link
Contributor

Bakual commented Mar 26, 2017

The drag zone is whole page of the Extensions -> Manage -> Install view and it works for 4 tabs.

Imho it should only work on the "Upload Package File" tab. To me it doesn't make much sense and could be confusing to be able to drop a file into the "Install by URL", "Install from Folder" or "Install from Web" tab.
Also, the code should be added by the plugin responsible for that tab (https://github.com/joomla/joomla-cms/blob/staging/plugins/installer/packageinstaller/tmpl/default.php) and not by com_installer.

@dgrammatiko
Copy link
Contributor

dgrammatiko commented Mar 26, 2017

@asika32764 can you move the code to the file form field (maybe with an option allowdnd) ?
This way we make it available everywhere, not only in this page

Ps: https://github.com/joomla/joomla-cms/blob/staging/layouts/joomla/form/field/file.php

@brianteeman
Copy link
Contributor

brianteeman commented Mar 26, 2017 via email

@brianteeman
Copy link
Contributor

@jeckodevelopment look at how its done in the tinymce plugin we already have for image uploads. You only get the box when you are dragging and dropping - not before

@Bakual
Copy link
Contributor

Bakual commented Mar 26, 2017

an you move the code to the file form field (maybe with an option allowdnd) ?
This way we make it available everywhere, not only in this page

It's an idea, but it would have to work also when multiple file fields are present in a form. You can't have the whole form be the trigger area in this case. You probably need an attribute which specifies the area identifier.

@dgrammatiko
Copy link
Contributor

dgrammatiko commented Mar 26, 2017

@Bakual I guess we can append (programmatically) some unique classes to the drop area and have the script loop through the classes and do it's magic

@asika32764
Copy link
Contributor Author

asika32764 commented Mar 26, 2017

Imho it should only work on the "Upload Package File" tab.

I didn't put code in plugin because I set whole page as the drag zone. And the upload from file function is hard coded in com_installer InstallerModelInstall, then the ajax upload must add a new task to controller, it is deep coupled to core installer component, so I decided to make the feature works for all tabs.

This concept also inspired by phpMyAdmin, it can drag file in any pages to import SQL file. (If someone always click the select file button, they won't notice this function, but if someone try to drag file to the small button, they will notice that there is a new feature...)

I can move code to plugin and make the drop zone only in install from upload tab content, but we need a box with dashed styling, I'm not sure how should I do this layout (what size? color? and etc...).

can you move the code to the file form field (maybe with an option allowdnd) ?
This way we make it available everywhere, not only in this page

@dgt41 It is a good idea for a drag & drop file field, but it is too big to me to implement this feature since I just got one night free to do this. Let's make this PR work first and leave the new field for future PRs.

@asika32764
Copy link
Contributor Author

@dgt41

About the drag & drop field, since Javascript is not able to inject file path to HTML file input, we have only two way that use ajax to send POST uploading or convert file to base64 and store in text field.

I have implemented this feature in my RAD framework, see:
default

But we must consider about this is not a standard way for JForm process, developer must manually handle the ajax or base64 string for every fields.

@asika32764
Copy link
Contributor Author

UPDATE

Based on discussion, moved code to plg_installer_packageinstaller and limited this function only work for Upload Install tab, also added the message to notice user.

I still set whole page as drag zone, whether we should create a dashed box depends on future discussion.

@ghost
Copy link

ghost commented Mar 27, 2017

@asika32764 lets chose the Way People expect: i guess its a dashed box, if this is best Practice.

.css("display", "none")
.css("margin-top", "-10px");
JoomlaInstaller.getLoadingOverlay()
.css("top", outerDiv.position().top - $(window).scrollTop())
Copy link
Member

Choose a reason for hiding this comment

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

Could you use tabs instead of spaces to indent these js and css?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fix in #14924

@asika32764
Copy link
Contributor Author

Create a new PR in #14924 that we can discuss for the new box layout.

@joomla-cms-bot
Copy link

Set to "closed" on behalf of @infograf768 by The JTracker Application at issues.joomla.org/joomla-cms/14905

@infograf768
Copy link
Member

Closed as we have new PR #14924


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

rdeutz pushed a commit that referenced this pull request Mar 30, 2017
* Add drag & drop installation

* Fix drag issue

* Add more error message

* Typo

* CS

* Style fix

* Fix from review

* Add return

* Add `.`

* Fix based on review

* Add return url

* Convert loading cover operation into object

* MISC fixes

* Fix indents

* Add box uploader

* CS

* Styling

* Move return val out

* Moved token out
@asika32764 asika32764 deleted the feature-drag-install branch July 29, 2017 15:57
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