Skip to content

Conversation

@newhinton
Copy link
Contributor

@newhinton newhinton commented Dec 7, 2018

closes #10908
This adds the ability to drop a folder on the quickaccess and add it to it

@newhinton newhinton self-assigned this Dec 7, 2018
@newhinton newhinton added enhancement design Design, UI, UX, etc. 3. to review Waiting for reviews labels Dec 7, 2018
@newhinton newhinton added this to the Nextcloud 16 milestone Dec 7, 2018
@newhinton newhinton changed the title Updated PR to nc16dev and fixed dropshadow Added Drozone to favorites quickaccess Dec 7, 2018
@newhinton
Copy link
Contributor Author

CI is failing, but i think the errors are unrelated:

Scenario: create a new user with a custom display name                       # /drone/src/github.com/nextcloud/server/tests/acceptance/features/users.feature:12
    Given I am logged in as the admin                                          # LoginPageContext::iAmLoggedInAsTheAdmin()
    And I open the User settings                                               # SettingsMenuContext::iOpenTheUserSettings()
    When I click the New user button                                           # UsersSettingsContext::iClickTheNewUserButton()
    And I see that the new user form is shown                                  # UsersSettintestgsContext::iSeeThatTheNewUserFormIsShown()
    And I set the user name for the new user to "test"                         # UsersSettingsContext::iSetTheUserNameForTheNewUserTo()
    And I set the display name for the new user to "Test display name"         # UsersSettingsContext::iSetTheDisplayNameForTheNewUserTo()
    And I set the password for the new user to "123456acb"                     # UsersSettingsContext::iSetThePasswordForTheNewUserTo()
    And I create the new user                                                  # UsersSettingsContext::iCreateTheNewUser()
    Then I see that the list of users contains the user "test"                 # UsersSettingsContext::iSeeThatTheListOfUsersContainsTheUser()
    And I see that the display name for the user "test" is "Test display name" # UsersSettingsContext::iSeeThatTheDisplayNameForTheUserIs()
      Row for user test in Users Settings could not be found after 100 seconds
      displayName cell for user test in Users Settings could not be found after 100 seconds
      displayName input for user test in Users Settings could not be found after 100 seconds (NoSuchElementException)
sh: 1: kill: No such process

and


Enable an app bundle                                          # /drone/src/github.com/nextcloud/server/tests/acceptance/features/apps.feature:42
    Given I act as Jane                                                   # ActorContext::iActAs()
    And I am logged in as the admin                                       # LoginPageContext::iAmLoggedInAsTheAdmin()
    And I open the Apps management                                        # SettingsMenuContext::iOpenTheAppsManagement()
    And I open the "App bundles" section                                  # AppNavigationContext::iOpenTheSection()
    When I enable all apps from the "Enterprise bundle"                   # AppsManagementContext::iEnableAllAppsFromThe()
    Then I see that the "Auditing / Logging" app has been enabled         # AppsManagementContext::iSeeThatTheAppHasBeenEnabled()
      Disable button in the app list for Auditing / Logging could not be found after 100 seconds (NoSuchElementException)
And I see that the "LDAP user and group backend" app has been enabled # AppsManagementContext::iSeeThatTheAppHasBeenEnabled()

Copy link
Member

@danxuliu danxuliu left a comment

Choose a reason for hiding this comment

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

Please rebase on current master :-) (then you will need to force push the branch again).

Tested and works, but it should be tested again once rebased ;-)

Maybe it would be good to not enable the favorite drop zone when dragging an already favorited folder, but that is a minor detail. In any case, a really nice feature, thanks :-D

$.event.trigger({type: "droppedOnTrash"});

var self=this;
var _self=this;
Copy link
Member

Choose a reason for hiding this comment

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

Why the rename?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Initially i had problems with self, it would not work properly. I can look if it works now

_self.do_delete(filename, directory);
});

$.event.trigger({type: "droppedOnFavorites"});
Copy link
Member

Choose a reason for hiding this comment

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

Why is this needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This deletes the folder when it is dropped on the trash

@danxuliu
Copy link
Member

CI is failing, but i think the errors are unrelated:

Yes, sh: 1: kill: No such process is a crash in the PHP built-in server (do not worry, you did not cause it :-P ), and the other failure was caused by an app not having been released yet in the app store when the tests were run (again, nothing that you need to worry about ;-) ).

@newhinton newhinton force-pushed the feature/noid/favorites-quickaccess-add-droppable-v3 branch 2 times, most recently from c4d4384 to 2c8a4b1 Compare January 13, 2019 20:24
@newhinton
Copy link
Contributor Author

@danxuliu rebased

@newhinton
Copy link
Contributor Author

oh, and removed the duplicate _self

danxuliu and others added 12 commits January 31, 2019 17:40
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 force-pushed the feature/noid/favorites-quickaccess-add-droppable-v3 branch from 2c8a4b1 to f13b3ab Compare February 1, 2019 01:12
@danxuliu
Copy link
Member

danxuliu commented Feb 1, 2019

@newhinton, sorry for the delay.

I have taken the liberty of rebasing again your pull request and do some cleaning of the drop zone feature and the pull request history. Please refer to each individual commit and its comment for further information.

Regarding the commit that adds the drop zone to the favorites I have removed .attr('data-favorite', elem.tags.includes("_$!<Favorite>!$_")) (which should have been .attr('data-favorite', elem.tags.includes(OC.TAG_FAVORITES)) ;-) ) because it was not needed with the current handling of the drop on the favorites and because, due to bugs in other parts of the Files code, elem may not always have a tags attribute (even if it should), which broke dragging rows in the file list (this could happen, for example, when using the Select all checkbox).

Copy link
Member

@danxuliu danxuliu 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 👍

Currently favoriting by dropping is limited to only one file or folder at a time due to some unrelated bugs in the Files app; I would merge this anyway in its current state.

Thanks again @newhinton!

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 30fb78c into master Feb 1, 2019
@MorrisJobke MorrisJobke deleted the feature/noid/favorites-quickaccess-add-droppable-v3 branch February 1, 2019 08:30
@newhinton
Copy link
Contributor Author

I didn't know that there was an OC.Tag introduced, i wrote a majority of this back at the conf, so there may have been some changes which im not aware of ^^ but good to know :D

@danxuliu
Copy link
Member

danxuliu commented Feb 1, 2019

No problem at all about that @newhinton, you did not have to know it; I just mentioned it for your information :-)

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 design Design, UI, UX, etc. enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants