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

UI Service: Dropzones #586

Merged

Conversation

wanze
Copy link
Contributor

@wanze wanze commented Jul 21, 2017

Proposal for the interfaces of file dropzones, allowing to drag and drop files and the upload process to the server. Two types of dropzones are introduced:

  • Standard: A visible dropzone containing a message and an additional link to manually select files from the computer. This dropzone is similar to the one currently existing when creating file objects.
  • Wrapper: This type of dropzone wraps around any other component of the UI framework. It is not visible by default but highlighted once files are dragged over the browser window. Dropped files are displayed in a round trip modal.

Please note: Although this pull request contains a working implementation, we encourage to mainly discuss the introduced interfaces here, due to the complexity of file uploads.

Factory Descriptions: https://github.com/ILIAS-eLearning/ILIAS/pull/586/files#diff-c0ddc616843072099030db8c4a168890R17

Common Interface for file drozpones: https://github.com/studer-raimann/ILIAS/blob/a4dc212d5715ea6758c26015af64cc8d06bccedb/src/UI/Component/Dropzone/File/File.php

Interface standard dropzone: https://github.com/studer-raimann/ILIAS/blob/a4dc212d5715ea6758c26015af64cc8d06bccedb/src/UI/Component/Dropzone/File/Standard.php

Interface wrapper dropzone: https://github.com/studer-raimann/ILIAS/blob/a4dc212d5715ea6758c26015af64cc8d06bccedb/src/UI/Component/Dropzone/File/Wrapper.php

Nicolas Märchy added 30 commits May 5, 2017 16:14
Copy link
Contributor

@Amstutz Amstutz left a comment

Choose a reason for hiding this comment

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

Thank you for this enhancement. I think there is a lot of brainfood in here that we can build up on in later components. Some aspects might certainly be refined as the framework evolves but I think think this is a big step forward.

See comments for minor corrections.

/* Copyright (c) 1998-2009 ILIAS open source, Extended GPL, see docs/LICENSE */

/**
* This class represents a file property in a property form.
Copy link
Contributor

Choose a reason for hiding this comment

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

Please describe here shortly the relation to the new UI service and why this class is needed.

/**
* @var int
*/
static $count = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Describe what this variable does here.

/**
* @var string
*/
protected $upload_url = '';
Copy link
Contributor

Choose a reason for hiding this comment

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

Describe what this variable does here.

/**
* @var int
*/
protected $max_files = 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

Describe what this variable does here.

/**
* @var \ILIAS\Data\DataSize
*/
protected $max_file_size;
Copy link
Contributor

Choose a reason for hiding this comment

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

Describe what this variable does here.

*
* @param {boolean} darkenedBackground true to use the darkened background for highlighting, otherwise false
*
* @private
Copy link
Contributor

Choose a reason for hiding this comment

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

Somehow clear, since they are not exposed, but .i.O..

Copy link
Member

Choose a reason for hiding this comment

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

This is another jsDoc thingy, will put that to the JS-Readme PR later

options.fileListContainer = $dropzone.find('.il-modal-roundtrip').find('.il-upload-file-list');
options.uploadButton = $dropzone.find('.modal-footer a.btn-primary:first');

console.log(options);
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove.

@@ -0,0 +1,85 @@
// 1.0.3
Copy link
Contributor

Choose a reason for hiding this comment

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

Please move to global bower libs as soon as accepted by JF and available in core, see other PR concerning this. If not accepted, this may stay.

Copy link
Member

Choose a reason for hiding this comment

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

Already prepared :-)

@il-dropzone-filelist-border-color: @il-dropzone-border-color;
@il-dropzone-filelist-border-type: solid;
@il-dropzone-filelist-border-width: @il-dropzone-border-width;
@il-dropzone-filelist-border-radius: @il-dropzone-border-radius;
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for this well chosen set of variables.

* Adds a html div to enable the darkened background, if the passed in argument is true.
* Sets the state of the darkened background availability to the value of the passed in argument.
*
* @param {boolean} darkenedBackground true, if the darkened background should be available
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this derived from some standard on how to add docComments on JS? if yes, adjust the readme of the UI-Framework.

Copy link
Member

Choose a reason for hiding this comment

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

same here, will open another PR

@chfsx
Copy link
Member

chfsx commented Aug 15, 2017

@Amstutz Thank you very much for your feedback I (hope) I tackled all of your comments with my last commits except of those concerning "withAdditionalOnLoadCode" which need some more time.

@chfsx
Copy link
Member

chfsx commented Aug 16, 2017

@Amstutz now the withAdditionalOnLoadCode() has also been implemented, thanks @klees for the hint! Some other small changes have been pushed. Thank you very much for the review

@Amstutz
Copy link
Contributor

Amstutz commented Aug 16, 2017

@klees still is there something missing for your approval? Note that if no further objections are voiced, this will be merged in the day.

Copy link
Member

@klees klees left a comment

Choose a reason for hiding this comment

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

Thx a lot, go on please =)

…accepted we will move the libraries out or /libs to /src/UI
@chfsx
Copy link
Member

chfsx commented Aug 16, 2017

I added the JS-libraries in the libs/bower directory, but if PR #603 won't be accepted we will move them out of this location to src/UI

@chfsx chfsx merged commit 0b8d424 into ILIAS-eLearning:trunk Aug 16, 2017
@chfsx chfsx deleted the feature/5-3/ui-service-dropzones-2 branch August 16, 2017 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants