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 and Drop Installation with upload box for #14905 #14924

Merged
merged 19 commits into from Mar 30, 2017

Conversation

asika32764
Copy link
Contributor

@asika32764 asika32764 commented Mar 27, 2017

Pull Request for Issue #14905

Summary of Changes

This is a new PR for #14905 that for easy discuss about the new upload layout.

Images

Drag and drop install

drag-upload

Select install

select-upload

If browser does not support FormData, will fallback to original upload layout. Please someone help me test in IE8.

@ghost
Copy link

ghost commented Mar 27, 2017

I have tested this item ✅ successfully on d66be84


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

@C-Lodder
Copy link
Member

All the JS and CSS should really go into separate files:

media/com_installer/js/com_installer.js
media/com_installer/css/com_installer.css

@asika32764
Copy link
Contributor Author

All the JS and CSS should really go into separate files.

May I just let the maintenance team to do this since I just follows original code to write it in php.

For me, it is make sense to write this css, js in plugin file because we can close them by closing plugin instead couple them in com_installer.

@infograf768
Copy link
Member

Agree with @C-Lodder

Also, I suggest that the dropbox be a bit different, something like

#dragarea {
    background-color: #fafbfc;
    border: 1px dashed #999;
    padding: 5% 0;
    transition: all 0.2s ease 0s;
    width: 100%;
}

It would give something like

screen shot 2017-03-27 at 16 17 07

What do you think?

@asika32764
Copy link
Contributor Author

asika32764 commented Mar 27, 2017

Not bad, I did the styling change, also added a box-sizing to align right-side.

@C-Lodder
Copy link
Member

@asika32764 - In J4, we're trying to move ALL Javascript and CSS to the media directory, so that it's all in 1 place. This means debugging and finding code is much easier. So it would be appreciated if you could do this.

The CSS bit is easy. As for the JS, you'll need to get the $return value, so simply add a hidden input like so:

<input type="hidden" id="installer-return" value="<?php echo $return; ?>">

Then get this value in the JS file:

var returnUrl = $('#installer-return');

@infograf768
Copy link
Member

infograf768 commented Mar 27, 2017

hmm, found an issue which is not related to this PR.

In LTR, we do get OK
screen shot 2017-03-27 at 16 44 10

But in RTL, it is another matter
screen shot 2017-03-27 at 16 45 14

Will create a specific PR
#14934

@asika32764
Copy link
Contributor Author

@C-Lodder

I know, my code is only against 3.7 now.

If I do this staff, I can only move my code since I don't know there may be any break after I moved other code wrote in the past and separated in different plugins. It will be clear that a PR just do one thing.

So I suggest make this thing happened after J4 that we can make sure code works currently in 3.7.

@C-Lodder
Copy link
Member

Ok leave it for now, I'll move it myself once this is merged and staging has been merged into 4.0-dev

@C-Lodder
Copy link
Member

I have tested this item ✅ successfully on 5cf5dce

Tested on:

@infograf768
Copy link
Member

I have tested this item ✅ successfully on 5cf5dce


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

@asika32764
Copy link
Contributor Author

asika32764 commented Mar 27, 2017

Moved return value out of JS from @C-Lodder 's suggest. That can be more easily to move code to single js file in the future.

@asika32764
Copy link
Contributor Author

asika32764 commented Mar 27, 2017

Also moved token out.

Maybe we can consider use Laravel way to handle CSRF token for Ajax and JS in J4: https://laravel.com/docs/5.4/csrf#csrf-x-csrf-token

@infograf768
Copy link
Member

I have tested this item ✅ successfully on 13630da


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

1 similar comment
@C-Lodder
Copy link
Member

I have tested this item ✅ successfully on 13630da


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

@jeckodevelopment
Copy link
Member

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Mar 28, 2017
@brianteeman
Copy link
Contributor

yay!!!!

@zero-24 zero-24 added this to the Joomla 3.7.0 milestone Mar 28, 2017
@rdeutz rdeutz merged commit e071996 into joomla:staging Mar 30, 2017
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Mar 30, 2017
@infograf768
Copy link
Member

@asika32764
Please look at https://issues.joomla.org/tracker/joomla-cms/15119

we have no message of success or error when using drag and drop

@asika32764 asika32764 deleted the feature-drag-install-box 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

8 participants