Create Folders when moving files by rummatee · Pull Request #13177 · nextcloud/server

rummatee · 2018-12-20T13:57:23Z

This Pull Request intends to solve Issue #7629 .
It adds a button to the file picker next to the breadscrumbs. When clicking on this button it opens a popup that asks for the folders name. After submitting the form, it closes the popup and reloads the file picker's content.
There is one small bug, that I couldn't fix: OC.hideMenus(); prevents the file picker from reacting to keyboard button presses until it is clicked again. So after creating a new folder the enter and escape key will only work again after the user clicked the file picker again.

Please review @MariusBluem @jancborchardt

jancborchardt · 2018-12-20T21:29:07Z

Great stuff @rummatee, cool that you work on these important design enhancements! Also cc @nextcloud/designers for review.

There is one small bug, that I couldn't fix: OC.hideMenus(); prevents the file picker from reacting to keyboard button presses until it is clicked again. So after creating a new folder the enter and escape key will only work again after the user clicked the file picker again.

Can anyone from @nextcloud/javascript help here?

jancborchardt · 2018-12-20T22:00:07Z

Nice! This seems to work well, only some feedback:

When navigating with the keyboard (via tab to switch elements and enter to "click"), pressing enter on the "New folder" button closes the dialog
The distance of the button to the end of the breadcrumbs is a bit much. Should be the same as in the Files app
When choosing a new folder name and pressing enter to confirm, the folder shows up twice or more initially (of course only exists once):
When you create a new folder, and then another one, the input placeholder should be reset to "New folder"

What do you think @rummatee? :)

rummatee · 2018-12-20T22:26:35Z

thanks for your feedback.

* The distance of the button to the end of the breadcrumbs is a bit much. Should be the same as in the Files app

I tried to change as little of the design of the file picker as possible. This also means that the button is slightly bigger than in the files app, as it uses the same height as the breadcrumbs. However I agree that a more uniform look would be better.

* When choosing a new folder name and pressing enter to confirm, the folder shows up twice or more initially (of course only exists once)

I totally missed this in my testing. Thank you for pointing it out.

* When you create a new folder, and then another one, the input placeholder should be reset to "New folder"

Yes, good point.

* When navigating with the keyboard (via tab to switch elements and enter to "click"), pressing enter on the "New folder" button closes the dialog

The keyboard control keeps being the trickiest part of this feature. Maybe opening the folder name prompt on pressing enter will also be difficult.

jancborchardt · 2018-12-21T22:56:17Z

Also @MariusBluem @fermulator for review since you were active in related issues. :)

kesselb · 2018-12-22T15:46:13Z

Button looks a bit off to me (Chromium 70+). Maybe related to your last changes because @jancborchardt screenshots looks ok or a local issue on my side.

kesselb

Good stuff 👍 Please align the button 😎

rummatee · 2018-12-23T10:34:44Z

Button looks a bit off to me (Chromium 70+). Maybe related to your last changes because @jancborchardt screenshots looks ok or a local issue on my side.

It seems to be related to Chrome. In Firefox it showed the position correctly. However my last commit should fix the position for both chrome and Firefox

jancborchardt · 2018-12-23T14:33:29Z

@nextcloud/designers can we have another review? :)

violoncelloCH · 2018-12-30T10:42:36Z

@rummatee Could this be rebased on current master? My dev setup instance complains that downgrading to 15.0.5 is unsupported, so I don't know how I should test this actually.

or how do you test this @jancborchardt @danielkesselberg ?

kesselb · 2018-12-30T13:26:31Z

I merged master into this branch locally.

violoncelloCH · 2018-12-30T15:36:52Z

thank you @danielkesselberg! this finally worked...

violoncelloCH

otherwise it looks and works great!

violoncelloCH · 2018-12-30T15:52:02Z

Also, if you create a folder in this "choose target folder" dialog, you normally want to place the file inside of this folder. So wouldn't it make sense to directly enter into the just created folder? What do you think @jancborchardt ?

jancborchardt · 2019-01-01T14:56:47Z

So wouldn't it make sense to directly enter into the just created folder?

@violoncelloCH Yeah, that would be a nice addition! :) Good suggestion!

@rummatee do you want to add that to this pull request, or want to do a separate one for that?

kesselb · 2019-01-01T16:44:14Z

Some tests are failing. Could you rebase this branch onto master?

rummatee · 2019-01-01T17:23:55Z

Some tests are failing. Could you rebase this branch onto master?

Did I do it correctly? I see that one test is still failing.

kesselb · 2019-01-01T17:28:28Z

Some tests are failing. Could you rebase this branch onto master?
Did I do it correctly? I see that one test is still failing.

Yes 👍 Litmus-Test is failing on master as well.

violoncelloCH

perfect from my view otherwise! 👍

kesselb · 2019-01-05T14:40:42Z

Another rebase should fix the litmus test. Not sure about the other tests. Restarted the build just in case.

kesselb · 2019-01-05T15:18:52Z

A different test fails now. I don't see how these changes are related to the failing test.

rummatee · 2019-01-05T17:28:53Z

I rebased again and again a different test is failing. I also can't see any relation to my changes.

jancborchardt · 2019-01-09T01:31:40Z

@MorrisJobke @rullzer could you check what it is about the failing tests? 👨‍🎤

MorrisJobke · 2019-01-09T09:55:16Z

@MorrisJobke @rullzer could you check what it is about the failing tests? 👨‍🎤

Those are all hiccups of our test suite 🙈 We should maybe get rid of them, because they are a major annoyance. cc @juliushaertl @danxuliu @rullzer

Signed-off-by: Florian Schunk <florian.schunk@rwth-aachen.de>

jancborchardt · 2019-01-09T15:58:11Z

@MorrisJobke ok, so in light of the reviews it would be fine to merge this? :)

jancborchardt · 2019-01-09T15:58:54Z

@rummatee please avoid force-pushing by the way as it makes it difficult to see which changes occured since the last time someone checked. :)

jancborchardt · 2019-01-09T16:00:08Z

Ah ok, now the test runs through, that’s why you force-pushed. Sorry and nevermind! :)

welcome · 2019-01-09T16:00:17Z

Thanks for your first pull request and welcome to the community! Feel free to keep them coming! If you are looking for issues to tackle then have a look at this selection: https://github.com/nextcloud/server/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22
Most developers hang out on IRC. So join #nextcloud-dev on Freenode for a chat!

rummatee · 2019-01-09T17:38:20Z

@rummatee please avoid force-pushing by the way as it makes it difficult to see which changes occured since the last time someone checked. :)

I'm supposed to rebase the pull request, right? Is there a different way to do it?

MorrisJobke · 2019-01-09T17:45:29Z

I'm supposed to rebase the pull request, right? Is there a different way to do it?

Typically if changes are added, then we do fixup commits and then a final rebase with --interactive --autosquash so that they will get squashed into one or multiple commits. That makes it usually easier. In this case if you want to get master stuff in the rebase is still fine IMO.

MorrisJobke · 2019-01-09T17:46:10Z

Ah ok, now the test runs through, that’s why you force-pushed. Sorry and nevermind! :)

That's why @jancborchardt also said that ^ 😉

jancborchardt · 2019-01-10T04:12:05Z

And @rummatee @danielkesselberg do you know about our next Contributor Week in Stuttgart, March 11–15? Contributors like you can apply for travel/accomodation support, find more info in the blog post at https://nextcloud.com/blog/the-contributor-week-is-over-time-to-plan-for-the-next-one/ 🙂

Would be really cool to have you! 🚀

jancborchardt requested review from kesselb and skjnldsv December 20, 2018 21:27

jancborchardt added enhancement 2. developing Work in progress feature: files feature: filepicker labels Dec 20, 2018

jancborchardt added this to the Nextcloud 16 milestone Dec 20, 2018

jancborchardt requested review from juliusknorr, kevgathuku and violoncelloCH December 20, 2018 21:29

jancborchardt requested a review from MariusBluem December 21, 2018 22:55

kesselb approved these changes Dec 22, 2018
View reviewed changes

violoncelloCH requested changes Dec 30, 2018
View reviewed changes

Comment threadcore/js/jquery.ocdialog.js Outdated
Comment threadcore/js/oc-dialogs.js Outdated

rummatee force-pushed the issue7629 branch from df4df73 to e749709Compare January 1, 2019 16:53

violoncelloCH requested changes Jan 1, 2019
View reviewed changes

Comment threadcore/js/oc-dialogs.js Outdated

rummatee force-pushed the issue7629 branch from cf11502 to 4cd773cCompare January 5, 2019 16:36

rummatee force-pushed the issue7629 branch 2 times, most recently from 1f15e4c to fea3104Compare January 8, 2019 10:42

rummatee added 10 commits January 9, 2019 11:28

added button for new folder to interface
7419746
Signed-off-by: Florian Schunk <florian.schunk@rwth-aachen.de>

add functionality to create new folder in file picker
dba14fa
Signed-off-by: Florian Schunk <florian.schunk@rwth-aachen.de>

adjust design to match file app
b61df4e
Signed-off-by: Florian Schunk <florian.schunk@rwth-aachen.de>

reset name of new folder and prevent the form from being submitted twice
ff1e0e2
Signed-off-by: Florian Schunk <florian.schunk@rwth-aachen.de>

open name prompt on pressing enter
b555fb3
Signed-off-by: Florian Schunk <florian.schunk@rwth-aachen.de>

change menu's orientation to make sure it is completly visible
808cf39
Signed-off-by: Florian Schunk <florian.schunk@rwth-aachen.de>

fix button position for chrome
0134ca5
Signed-off-by: Florian Schunk <florian.schunk@rwth-aachen.de>

use triple equal signs according to style guide
dd0b0d8
Signed-off-by: Florian Schunk <florian.schunk@rwth-aachen.de>

move to new folder
2fa36a7
Signed-off-by: Florian Schunk <florian.schunk@rwth-aachen.de>

use triple equal signs according to style guide
2cebdbb
Signed-off-by: Florian Schunk <florian.schunk@rwth-aachen.de>

rummatee force-pushed the issue7629 branch from fea3104 to 2cebdbbCompare January 9, 2019 10:29

jancborchardt merged commit 707cb68 into nextcloud:masterJan 9, 2019

This was referenced Mar 25, 2019
Fix filepicker's add button design #14806
Merged
hide new folder button in choose type filepicker #14900
Merged

Uh oh!

Uh oh!

Conversation

rummatee commented Dec 20, 2018

Uh oh!

jancborchardt commented Dec 20, 2018

Uh oh!

jancborchardt commented Dec 20, 2018

Uh oh!

rummatee commented Dec 20, 2018

Uh oh!

jancborchardt commented Dec 21, 2018

Uh oh!

kesselb commented Dec 22, 2018

Uh oh!

kesselb left a comment

Choose a reason for hiding this comment

Uh oh!

rummatee commented Dec 23, 2018

Uh oh!

jancborchardt commented Dec 23, 2018

Uh oh!

violoncelloCH commented Dec 30, 2018• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kesselb commented Dec 30, 2018

Uh oh!

violoncelloCH commented Dec 30, 2018• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

violoncelloCH left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

violoncelloCH commented Dec 30, 2018

Uh oh!

jancborchardt commented Jan 1, 2019• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kesselb commented Jan 1, 2019

Uh oh!

rummatee commented Jan 1, 2019

Uh oh!

kesselb commented Jan 1, 2019

Uh oh!

violoncelloCH left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kesselb commented Jan 5, 2019

Uh oh!

kesselb commented Jan 5, 2019• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rummatee commented Jan 5, 2019

Uh oh!

jancborchardt commented Jan 9, 2019

Uh oh!

MorrisJobke commented Jan 9, 2019

Uh oh!

jancborchardt commented Jan 9, 2019

Uh oh!

jancborchardt commented Jan 9, 2019

Uh oh!

jancborchardt commented Jan 9, 2019

Uh oh!

welcomeBot commented Jan 9, 2019

Uh oh!

rummatee commented Jan 9, 2019

Uh oh!

MorrisJobke commented Jan 9, 2019

Uh oh!

MorrisJobke commented Jan 9, 2019

Uh oh!

jancborchardt commented Jan 10, 2019• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

violoncelloCH commented Dec 30, 2018•
edited
Loading

violoncelloCH commented Dec 30, 2018•
edited
Loading

jancborchardt commented Jan 1, 2019•
edited
Loading

violoncelloCH left a comment •
edited
Loading

kesselb commented Jan 5, 2019•
edited
Loading

jancborchardt commented Jan 10, 2019•
edited
Loading