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

Nextcloud blue upload bar + Notifications #38

Closed
wants to merge 5 commits into from
Closed

Nextcloud blue upload bar + Notifications #38

wants to merge 5 commits into from

Conversation

williambargent
Copy link
Member

@williambargent williambargent commented Jun 10, 2016

  • Styles the upload bar in the nextcloud blue.
  • Changes notification background colour to white.
  • Adds faint border around file previews.
  • Removes border radius form dropdown navigation (Looks cleaner)

Fix for #35 @Mar1u5 @nextcloud/designers

@williambargent williambargent added bug design Design, UI, UX, etc. labels Jun 10, 2016
@@ -293,6 +293,7 @@ table td.filename .thumbnail {
float: left;
position: absolute;
z-index: 4;
border: 1px solid #f7f7f7;
Copy link
Member

Choose a reason for hiding this comment

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

please fix the indentation

@vincchan
Copy link
Member

@williambargent thanks for your contribution :) As a side note since you are a member of the nextcloud organisation you are able to create branches directly under the server repo. This enables reviewers to easily checkout your branch and your changes.

@vincchan
Copy link
Member

@williambargent also 6 commits for 4 lines of changes seems to be a bit overkill. Could you please rebase?

background: #0082c9 url('images/ui-bg_flat_35_1d2d44_40x100.png') 50% 50% repeat-x;
color: #ffffff;
background: #0082c9;
color: #0082c9;
Copy link
Member

@jancborchardt jancborchardt Jun 10, 2016

Choose a reason for hiding this comment

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

The background and text color is the same here. The color should be #fff white though.

@jancborchardt
Copy link
Member

Hey, so these are quite a lot of unrelated changes for one pull request. I’d say it would be good to separate these since one fixes an issue, one is a good enhancement, and the other two I would rather not change. :) Detailed comments:

  • Styles the upload bar in the nextcloud blue

Great! That fixes #35

  • Changes notification background colour to white.

That’s a actually a good enhancement to make the look nicer! For the future it would be good to separate this change out because it has nothing to do with the other things.

  • Adds faint border around file previews.

What’s the reasoning behind this change?

  • Removes border radius form dropdown navigation (Looks cleaner)

The rounded corner looks cleaner and friendlier than a sharp corner. I can recommend reading http://www.uiandus.com/blog/2009/7/27/realizations-of-rounded-rectangles.html :)

@williambargent
Copy link
Member Author

williambargent commented Jun 10, 2016

screenshot 2

The file table, navigation and previews are all line and corners, I felt it looked nicer rather than suddenly jumping to curves and where some previews have white backgrounds a faint border finished of the icon (The .exe is an example)

The reason there are a few different things in this request is, I didn't realize if I pushed to origin they would be added to an open pull request. Still getting used to git.

@williambargent williambargent changed the title Nextcloud blue upload bar Nextcloud blue upload bar + Notifications Jun 10, 2016
William Bargent added 2 commits June 10, 2016 14:38
Fix Indentation

Fix Indentation

fix font colour
@jancborchardt
Copy link
Member

@williambargent no worries. :) The reason there are no borders around the icon is that it’s unneeded visual noise. And the reason for the border-radius on the bottom of the menu dropdowns is that it’s a tiny detail fitting with the other round corners in the interface (logo, round avatars, all icons such as filetypes).

@jancborchardt
Copy link
Member

If you remove the border around the icon, and the border-radius change then we can review again. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug design Design, UI, UX, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants