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

Combine upload action into the "New" menu #18630

Merged
merged 4 commits into from
Sep 7, 2015

Conversation

PVince81
Copy link
Contributor

Combine upload action into the "New" menu.

Also refactored the new menu to be encapsulated in the more portable NewFileMenu JS class.
Moved directory and file creation logic to the FileList class instead of having it floating in file-upload.js.

This is a simplified version of #18589 where the menu stays where it is. We can move it to the sidebar later.

  • TODO: write JS unit tests

@jancborchardt @MorrisJobke please review.
If we agree on the design/placement I'll write the missing unit tests.

@PVince81 PVince81 added this to the 8.2-current milestone Aug 28, 2015
Refactored the new menu to be encapsulated in the NewFileMenu JS class
@PVince81
Copy link
Contributor Author

Let me rebase this onto master first so we get the latest styles.

@PVince81
Copy link
Contributor Author

Rebased. (Jenkins will stumble and fall)

@MorrisJobke
Copy link
Contributor

bildschirmfoto von 2015-08-28 17-53-25

Text file icon is missing.

@karlitschek
Copy link
Contributor

where are we with that? We need this for 8.2

@jancborchardt
Copy link
Member

Fixed some layout issues.

@PVince81 when I click New file or Folder, the input field does not show right away, but this:
capture du 2015-09-03 00-58-22
As soon as that is fixed, I can do the input field detail positioning.

@PVince81
Copy link
Contributor Author

PVince81 commented Sep 3, 2015

Strange, works fine in Chromium but Firefox shows the three dots

@PVince81
Copy link
Contributor Author

PVince81 commented Sep 3, 2015

There's a text-overflow: ellipsis on every menu item. For some reason it kicks in in Firefox but not Chromium. I'll disable it for this specific menu.

Last time I checked that all possible translations of "New file" would fit the current width, so it should be fine.

@PVince81
Copy link
Contributor Author

PVince81 commented Sep 3, 2015

@jancborchardt I removed the ellipsis, should work now.

I'll take care of unit tests.

- added unit tests for NewFileMenu
- use generateUrl for FileList.createFile and FileList.createDirectory
- added unit tests for FileList.createFile and FileList.createDirectory
@scrutinizer-notifier
Copy link

A new inspection was created.

@PVince81
Copy link
Contributor Author

PVince81 commented Sep 3, 2015

Unit tests were added.

@jancborchardt when you're done with the design, please set the label to "to review".
Thanks.

@karlitschek
Copy link
Contributor

Can we review and merge this? Quite important :-)

@MorrisJobke
Copy link
Contributor

Tested and works 👍

@jancborchardt
Copy link
Member

Nice! :) 👍 any details we can fix in follow-up PRs.

jancborchardt added a commit that referenced this pull request Sep 7, 2015
Combine upload action into the "New" menu
@jancborchardt jancborchardt merged commit 004de14 into master Sep 7, 2015
@jancborchardt jancborchardt deleted the files-combineuploadbutton branch September 7, 2015 12:03
@karlitschek
Copy link
Contributor

nice!

@lock lock bot locked as resolved and limited conversation to collaborators Aug 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants