Conversation
icewind1991
commented
Aug 9, 2016
- Hide "global credentails" and dependency notes when a user can't create mounts
- hide "External Storage" from personal settings if there is nothing to show
|
@icewind1991, thanks for your PR! By analyzing the annotation information on this pull request, we identified @jancborchardt, @Xenopathic and @PVince81 to be potential reviewers |
| var mainForm = $('#files_external'); | ||
| if (result.length === 0 && mainForm.attr('data-can-create') === 'false') { | ||
| mainForm.hide(); | ||
| $('a[href="#external-storage"]').parent().hide(); |
There was a problem hiding this comment.
This works but is a little bit ugly because the time that loading takes is around 0.5 seconds here and then the navigation bar on the left magically removes one item.
There was a problem hiding this comment.
Not much we can easily do about it
There was a problem hiding this comment.
Just a idea by @nickvergessen, shouldn't it be possible to already decide on PHP level if we want to show the settings or not and don't return a settings page here https://github.com/nextcloud/server/blob/master/apps/files_external/personal.php if we don't want to show the settings?
|
Works but https://github.com/nextcloud/server/pull/787/files#r74160076 makes it a little bit of a bad UX. |
a488b94 to
1522beb
Compare
|
I fixed some casing stuff too. This addresses some issues of #735 |
Agree... So what should we do here? How to move forward? @nextcloud/designers opinions? |
|
Should we split it up and first merge the uncontroversial part like fix whitespace, non italic text, "using OC login", no note,... And put the "hide if empty" part in a new PR for further discussion so that we have at least something for the RC? |
1522beb to
8f751c7
Compare
|
It now hides the "Files External" section on the server side |
|
Works great, @icewind1991 ! Just one small thing I discovered while testing. The drop-down to select the external storage has a option "ownCloud". I would suggest to change it to "Nextcloud / ownCloud". |
|
Changes it to "Nextcloud", showing both only adds clutter imo |
|
👍 |
dcdc6b3 to
9a7193c
Compare
|
I rebased because of the conflict (was only a string change in a file that was moved for the admin page split PR - #796) I tested it and it works 👍 |
|
👍 |
|
|
|
please backport :-) |
|
On it 💪 |
|
Backport is in #902 👊 |
|
We lost at least two commits, probably during a rebase.
@icewind1991 can you have a look and bring this commits back? Maybe you have a local copy? |
…gs-polish Files external settings polish