Change app menu to white background with dark icons #627
Conversation
juliusknorr
added
enhancement
design
|
@juliushaertl, thanks for your PR! By analyzing the annotation information on this pull request, we identified @jancborchardt, @ChristophWurst and @MorrisJobke to be potential reviewers |
|
The icons don't show in IE and only show half in Edge.
|
| @@ -252,8 +257,8 @@ | |||
| height: 32px; | |||
| } | |||
| #navigation .app-loading .app-icon { | |||
| -ms-filter: "progid:DXImageTransform.Microsoft.Alpha(Opacity=10)"; | |||
| opacity: .1; | |||
| -ms-filter: "progid:DXImageTransform.Microsoft.Alpha(Opacity=0)"; | |||
There was a problem hiding this comment.
No, as the icon should not be visible anymore, when the loading indicator is shown.
Ok, thanks, will try to fix that later today. |
MorrisJobke
added
2. developing
|
@juliushaertl awesome! Only thing from my side is that it looks a bit harsh – the icons could be lighter. Here it is with active changed from .8 to .6 and the others from .6 to .4: (We should also do that in the users menu on the top right then :) @juliushaertl if it’s ok I can add those commits, or you can feel free to do it. Don’t want to mess with your branch while you’re working on it. ;) |
|
@jancborchardt Right, that looks far nicer. Feel free to commit, otherwise I could also do that later this evening. |
| -moz-filter: invert(100%); | ||
| -o-filter: invert(100%); | ||
| -ms-filter: "progid:DXImageTransform.Microsoft.Invert(100%)"; | ||
| filter: invert(100%); |
There was a problem hiding this comment.
@juliushaertl please always use tabs ;) according to our coding style guidelines and what’s used elsewhere in the file.
|
@juliushaertl made the changes. Also, you need to use tabs instead of spaces ;D |
juliusknorr
force-pushed
the
white-app-menu
branch
from
7be1296
to
3194494
Compare
|
Pushed a new commit that now also supports IE 9+, Edge. Please review again @jancborchardt @nickvergessen @williambargent |
juliusknorr
added
3. to review
|
This looks really nice, great work! I just have a small issue. It seems like the "App" menu entry is handled differently then all the other app icons. The icon of the current active app is darker and the other icons change to the dark color on hover, expect the "apps" icon (there is also a hover effect but not as strong as for the other apps). If I go to the app settings the apps icon also doesn't change to the darker color by default. Tested with Chromium 52 |
|
@schiessle this is by design because the »Apps« entry is different from the others. It’s styled a bit more transparent accordingly to not be confused with an app. |
|
@williambargent mind retesting this? :) |
Hm, but if they are not selected all look the same, including the app menu. I consider it rather confusing that the hover effects behaves different on some elements on the same menu and especially confusing that I don't have a visual feedback that the current page is the app menu. |
|
How about keeping the Apps icon a bit lighter but changing the active state to the same grey as the other icons? @jancborchardt @schiessle |
|
Awesome, its almost working. When you hover over 'add apps' in Edge the colour doesn't change but does in IE. Also when your on the 'add apps' page it doesn't show that it has been selected. |
|
@williambargent That's what I already mentioned here: #627 (comment) imho the app menu should behave the same as any other entry with respect to the hover effect and the look if it is selected. Might be OK if it is a bit lighter if it is not selected to highlight the real apps a bit. But hover effect and selected color should be exactly the same. |
@juliushaertl sounds perfect! Fixed that :) |
|
@jancborchardt looks good :) |
|
Hm, I still see no visible hint that I'm at the apps page:
|
|
@schiessle try hard-refreshing (Ctrl-Shift-R) or incognito mode: |
|
Works for me 👍 |
Did all that but I should have done a git pull before testing it again 😉 Looks good now 👍 |
|
I would suggest to backport this for Nextcloud 10, objections? |
|
Definetly. 👍 What do you think? @karlitschek |
|
looks good. please backport 👍 |
|
@juliushaertl do you want to open a backport PR of your awesome work to stable10? :) Probably directly with #664 on top. |
|
@jancborchardt just did at #665 ;) |
Change app menu to white background with dark icons
This PR changes the app menu to white background with dark icons, as proposed in #455
Chrome, Firefox and Safari seem to work fine, could someone please verify that the inverting of icons works with IE/Edge?
Please review @jancborchardt @nextcloud/designers @Bugsbane @icewind1991 @williambargent