Skip to content

Conversation

@danxuliu
Copy link
Member

@danxuliu danxuliu commented Feb 1, 2019

Backport of the code cleanups (for easier future backports) and the bug fix (but not the new feature) of #12917

How to test:

  • Open the files app
  • Create a new folder
  • Drag the folder to the trash bin in the navigation bar

Result with this pull request:
The background of the trash bin row is highlighted.

Result without this pull request:
The background of the trash bin row is not highlighted, so no feedback is given to the user to know that the folder can be dropped in the trash bin.

danxuliu and others added 11 commits February 1, 2019 16:21
Since 6ad7f32 SVG icons are directly embedded in "icons-vars.css", so
the starred trash icon is now loaded along with the regular trash icon
all at once. Therefore it is not needed to explicitly prefetch it using
a hidden div.

Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Signed-off-by: Daniel Calviño Sánchez <[email protected]>
The "droppedOnTrash" event was being triggered when the file list was
initialized, but it should be triggered only when the user actually
drops a file on the trash bin.

Besides that, the event had no effect; only the file list handles it,
but as it was not triggered on any element it ended being triggered on
the document, and thus not handled. Moreover, even if it had been
triggered on the file list it would have been done before the handler
was set, so it would not have been handled anyway. And even if it had
been handled no data was provided, so the handler would have failed.

In conclusion, triggering the event there was not needed, and thus it
was removed.

Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Signed-off-by: Daniel Calviño Sánchez <[email protected]>
When a single file was dropped on the trash bin the file information was
gotten from the original element in the file list. When several files
were dropped on the trash bin the file information was gotten from the
helper elements being dragged around. The helper element also contain
the needed file information when a single file is being dragged, so the
handling was unified to always get the file information from the helper
elements.

As the handling of several files is the same as before there is still
the issue of only deleting those files shown in the drag helper instead
of all the selected files.

Signed-off-by: Daniel Calviño Sánchez <[email protected]>
@danxuliu danxuliu added bug design Design, UI, UX, etc. 3. to review Waiting for reviews labels Feb 1, 2019
@danxuliu danxuliu added this to the Nextcloud 15.0.4 milestone Feb 1, 2019
rullzer
rullzer previously requested changes Feb 1, 2019
Copy link
Member

@rullzer rullzer left a comment

Choose a reason for hiding this comment

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

blocked till 15.0.3 is out

Copy link
Member

@rullzer rullzer left a comment

Choose a reason for hiding this comment

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

Quick smoke tests was ok here

Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

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

Tested and works 👍

@MorrisJobke MorrisJobke merged commit 6ae8e99 into stable15 Feb 8, 2019
@MorrisJobke MorrisJobke deleted the stable15-12917-clean-code-and-fix-drop-zone-shadow branch February 8, 2019 08:34
@MorrisJobke MorrisJobke mentioned this pull request Feb 20, 2019
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews bug design Design, UI, UX, etc.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants