-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Conversation
@@ -293,6 +293,7 @@ table td.filename .thumbnail { | |||
float: left; | |||
position: absolute; | |||
z-index: 4; | |||
border: 1px solid #f7f7f7; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please fix the indentation
@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. |
@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; |
There was a problem hiding this comment.
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.
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:
Great! That fixes #35
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.
What’s the reasoning behind this change?
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 :) |
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. |
Fix Indentation Fix Indentation fix font colour
@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). |
If you remove the border around the icon, and the border-radius change then we can review again. :) |
Fix for #35 @Mar1u5 @nextcloud/designers