External storage design fixes and update #8496

skjnldsv · 2018-02-22T11:18:51Z

Before	After

Remove the dropdown class and use a popovermenu
Add select possibility design for popovermenu
Change error/save feedback as it is not very comprehensible right now

Signed-off-by: John Molakvoæ (skjnldsv) <[email protected]>

skjnldsv · 2018-02-22T11:23:21Z

core/css/apps.scss

-	border-bottom-left-radius: 5px;
-	border-bottom-right-radius: 5px;
-	box-shadow: 0 1px 1px $color-box-shadow;
+	background: $color-main-background;


Hum, this should not have been pushed.

codecov · 2018-02-22T14:04:44Z

Codecov Report

❗ No coverage uploaded for pull request base (master@3c30f29). Click here to learn what that means.
The diff coverage is 45.45%.

@@            Coverage Diff            @@
##             master    #8496   +/-   ##
=========================================
  Coverage          ?   51.81%           
  Complexity        ?    25397           
=========================================
  Files             ?     1603           
  Lines             ?    95211           
  Branches          ?     1376           
=========================================
  Hits              ?    49335           
  Misses            ?    45876           
  Partials          ?        0

Impacted Files	Coverage Δ	Complexity Δ
apps/files_external/templates/settings.php	`0% <0%> (ø)`	`0 <0> (?)`
apps/files_external/js/settings.js	`58.3% <62.5%> (ø)`	`0 <0> (?)`

skjnldsv · 2018-02-22T14:43:22Z

Ready for reviews! @nextcloud/designers
@MorrisJobke can you add the people in charge of the ext storage as reviewers please? :)

Signed-off-by: John Molakvoæ (skjnldsv) <[email protected]>

pixelipo · 2018-02-22T15:11:54Z

apps/files_external/js/settings.js

 // TODO: move to a separate file
 var MOUNT_OPTIONS_DROPDOWN_TEMPLATE =
-	'<div class="drop dropdown mountOptionsDropdown">' +
-	// FIXME: options are hard-coded for now


IMO this comment has not been addressed in this PR - options are still hard-coded. I would suggest either keeping the comment or creating a generator.

pixelipo · 2018-02-22T15:13:44Z

settings/css/settings.scss

 span {
 	&.success {
-		background: #37ce02;
+		background-color: $color-success;


nice catch!

pixelipo · 2018-02-22T15:19:43Z

I'm not sure about the new status icons - I have to say I'm quite partial to the obviousness and simplicity of the old ones :) Also, new once seem to have a non-standard size - at lease as far as their padding is concerned. 28px? 16/20 (icon size) and 44/50 (container) those are sizes we use, right?

skjnldsv · 2018-02-22T15:23:58Z

Yeah, I'm not really convinced either, but it's already a great improvement compared to the old ones ^^
Yes, they're all with a 28px width/height. At first I started using only the grey icons, but it was confusing (especially with the checkmark) so I decided to go for something entirely different which, at least, looks pretty clear.

What do you suggest @pixelipo ? :)

enoch85 · 2018-02-22T15:33:53Z

Does the user get any output when hoovering on the icons on what's wrong?

enoch85 · 2018-02-22T15:34:39Z

And regarding the icons, maybe we should use the same one as in with the Desktop Client?

skjnldsv · 2018-02-22T15:35:52Z

@enoch85 it depends on what the server returns. Either a popup saying what's wrong or nothing at all :/

pixelipo

Padding issues with the icon buttons:

Row should be 50px high:

pixelipo · 2018-02-22T15:53:12Z

sorry, @skjnldsv ! I know I can be such a pain in the **** sometimes...

skjnldsv · 2018-02-22T15:55:31Z

@pixelipo don't apologise!! I wasn't blaming you!! 🤗
I'll fix it after my work on the update notifications to vue.

Signed-off-by: John Molakvoæ (skjnldsv) <[email protected]>

skjnldsv · 2018-02-26T12:04:00Z

All clear @pixelipo

Please update

MorrisJobke · 2018-02-26T17:25:38Z

@skjnldsv Ready for review? Then update the labels ;)

skjnldsv · 2018-02-26T17:41:36Z

Oups! :)

MorrisJobke · 2018-02-27T10:27:47Z

I can't click the gear, trashbin or checkmark icon - there doesn't happen anything (cleared browser cache and tried Safari and Chrome)
a little UI papercut: the checkmark has no top border:

Signed-off-by: John Molakvoæ (skjnldsv) <[email protected]>

skjnldsv · 2018-02-27T10:35:44Z

@MorrisJobke Indeed, it's fixed. Forgot to update a section of my code :)

MorrisJobke

Looks awesome and works 👍

MorrisJobke · 2018-02-27T11:02:25Z

JSUnit tests for files_external fail now

Signed-off-by: John Molakvoæ (skjnldsv) <[email protected]>

pixelipo

Looks good to me

Signed-off-by: John Molakvoæ (skjnldsv) <[email protected]>

Use popovermenu instead of dropdown and small fix to popovermenu

1e2a082

Signed-off-by: John Molakvoæ (skjnldsv) <[email protected]>

skjnldsv requested a review from MorrisJobke February 22, 2018 11:20

skjnldsv self-assigned this Feb 22, 2018

skjnldsv added enhancement design Design, UI, UX, etc. 2. developing Work in progress low feature: external storage papercut Annoying recurring UX issue with possibly simple fix. labels Feb 22, 2018

skjnldsv added this to the Nextcloud 14 milestone Feb 22, 2018

skjnldsv commented Feb 22, 2018

View reviewed changes

skjnldsv added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Feb 22, 2018

skjnldsv added 6 commits February 22, 2018 15:45

fixup! Use popovermenu instead of dropdown and small fix to popovermenu

3d2b3a1

Signed-off-by: John Molakvoæ (skjnldsv) <[email protected]>

Changed config saving feedback design

0cc520f

Signed-off-by: John Molakvoæ (skjnldsv) <[email protected]>

Better label styling

884605a

Signed-off-by: John Molakvoæ (skjnldsv) <[email protected]>

fixup! Changed config saving feedback design

1f3eef9

Signed-off-by: John Molakvoæ (skjnldsv) <[email protected]>

Tooltip fix on status icon

323dd31

Signed-off-by: John Molakvoæ (skjnldsv) <[email protected]>

Select in popover fix

dab83c4

Signed-off-by: John Molakvoæ (skjnldsv) <[email protected]>

skjnldsv force-pushed the ext-strg-design-fixes branch from b583266 to dab83c4 Compare February 22, 2018 14:45

pixelipo reviewed Feb 22, 2018

View reviewed changes

settings/css/settings.scss

span {

&.success {

background: #37ce02;

background-color: $color-success;

Copy link

Contributor

pixelipo Feb 22, 2018

Choose a reason for hiding this comment

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

nice catch!

pixelipo previously requested changes Feb 22, 2018

View reviewed changes

skjnldsv removed the 3. to review Waiting for reviews label Feb 22, 2018

skjnldsv added 2 commits February 26, 2018 13:02

Fixed icons padding

da2aafa

Signed-off-by: John Molakvoæ (skjnldsv) <[email protected]>

fixup! Fixed icons padding

0f229d2

Signed-off-by: John Molakvoæ (skjnldsv) <[email protected]>

skjnldsv requested review from danxuliu, enoch85, jancborchardt, juliusknorr and rullzer February 26, 2018 17:12

skjnldsv added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Feb 26, 2018

Fixed actions and header missing th

6b45d7e

Signed-off-by: John Molakvoæ (skjnldsv) <[email protected]>

MorrisJobke approved these changes Feb 27, 2018

View reviewed changes

skjnldsv added 2 commits February 27, 2018 12:11

Fixed tests

324e96b

Signed-off-by: John Molakvoæ (skjnldsv) <[email protected]>

fixup! Fixed tests

f1634cd

Signed-off-by: John Molakvoæ (skjnldsv) <[email protected]>

pixelipo approved these changes Feb 27, 2018

View reviewed changes

skjnldsv added 2 commits February 27, 2018 13:05

fixup! Fixed tests

6ee9a2b

Signed-off-by: John Molakvoæ (skjnldsv) <[email protected]>

fixup! Fixed tests

5f217e2

Signed-off-by: John Molakvoæ (skjnldsv) <[email protected]>

skjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Feb 27, 2018

MorrisJobke merged commit 7792744 into master Feb 27, 2018

MorrisJobke deleted the ext-strg-design-fixes branch February 27, 2018 14:59

Uh oh!

External storage design fixes and update #8496

External storage design fixes and update #8496

Uh oh!

Conversation

skjnldsv commented Feb 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

skjnldsv Feb 22, 2018

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Feb 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

skjnldsv commented Feb 22, 2018

Uh oh!

pixelipo Feb 22, 2018

Choose a reason for hiding this comment

Uh oh!

pixelipo Feb 22, 2018

Choose a reason for hiding this comment

Uh oh!

pixelipo commented Feb 22, 2018

Uh oh!

skjnldsv commented Feb 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

enoch85 commented Feb 22, 2018

Uh oh!

enoch85 commented Feb 22, 2018

Uh oh!

skjnldsv commented Feb 22, 2018

Uh oh!

pixelipo left a comment

Choose a reason for hiding this comment

Uh oh!

pixelipo commented Feb 22, 2018

Uh oh!

skjnldsv commented Feb 22, 2018

Uh oh!

skjnldsv commented Feb 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MorrisJobke commented Feb 26, 2018

Uh oh!

skjnldsv commented Feb 26, 2018

Uh oh!

MorrisJobke commented Feb 27, 2018 • edited by skjnldsv Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

skjnldsv commented Feb 27, 2018

Uh oh!

MorrisJobke left a comment

Choose a reason for hiding this comment

Uh oh!

MorrisJobke commented Feb 27, 2018

Uh oh!

pixelipo left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

skjnldsv commented Feb 22, 2018 •

edited

Loading

codecov bot commented Feb 22, 2018 •

edited

Loading

skjnldsv commented Feb 22, 2018 •

edited

Loading

skjnldsv commented Feb 26, 2018 •

edited

Loading

MorrisJobke commented Feb 27, 2018 •

edited by skjnldsv

Loading