add icons to navigation of personal & admin settings#3151
add icons to navigation of personal & admin settings#3151MorrisJobke merged 11 commits intomasterfrom
Conversation
jancborchardt
added
3. to review
|
Looks really nice, but the serverinfo app is missing an icon:
|
MorrisJobke
left a comment
MorrisJobke
left a comment
There was a problem hiding this comment.
Looks nice, but we maybe should add the entry for the serverinfo app. I also added the viewBox elements where they were missing. 😉
|
I like it a lot too :) |
settings/css/settings.css
Outdated
| .nav-icon-workflow { | ||
| background-image: url('../img/tag.svg?v=1'); | ||
| } | ||
| .nav-icon-survey_client { |
There was a problem hiding this comment.
This is cheating and should be part of the api instead, otherwise apps that add their own sections dont have one....
I can do that...
There was a problem hiding this comment.
This should probably be in icons.scss.
There was a problem hiding this comment.
@nickvergessen yup, that's why I mentioned it would be nicer to get it from the app! :) Would be cool if you can do that.
I looked in the activity app also but there you din't use the standard way with the ul class="with-icon" and instead of background-image use img elements. If possible we should use the standard as styled in the apps.css.
Signed-off-by: Jan-Christoph Borchardt <hey@jancborchardt.net>
Signed-off-by: Jan-Christoph Borchardt <hey@jancborchardt.net>
Signed-off-by: Morris Jobke <hey@morrisjobke.de>
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
nickvergessen
force-pushed
the
navigation-icons
branch
from
0eb8969 to
17a7eaa
Compare
|
I fixed the admin section, so the icons are now part of the API. |
nickvergessen
added
2. developing
|
Setting to develop because test will fail |
Signed-off-by: Joas Schilling <coding@schilljs.com>
| margin-bottom: -3px; | ||
| margin-right: 6px; | ||
| width: 16px; | ||
| } |
There was a problem hiding this comment.
Yeah, I actually surrendered on nextcloud/nextcloud.com#140 for now. We need to find a more engaging, agile way to work on this. Let's talk @ FOSDEM about that...
|
@nickvergessen awesome! Just one thing: We should use the same method as we specified in the apps.css, which is the one we use in Files and Mail etc. Now there’s a discrepancy in method, and this even results in the spacing being different from the one in Files. |
|
@jancborchardt Well activity uses image-paths as well and we need that when we want to make use of them in android/desktop apps as well. |
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Morris Jobke <hey@morrisjobke.de>
MorrisJobke
added
3. to review
|
Drone stalled on 5.6 again. I restarted the tests. 🙈 🙉 🙊 |
|
👍 |
* followup to #3151 Signed-off-by: Morris Jobke <hey@morrisjobke.de>
* follow up to nextcloud/server#3151 Signed-off-by: Morris Jobke <hey@morrisjobke.de>
* follow up to nextcloud/server#3151 Signed-off-by: Morris Jobke <hey@morrisjobke.de>
* follow up to nextcloud/server#3151 Signed-off-by: Morris Jobke <hey@morrisjobke.de>
* follow up to nextcloud/server#3151 Signed-off-by: Morris Jobke <hey@morrisjobke.de>
* follow up to nextcloud/server#3151 Signed-off-by: Morris Jobke <hey@morrisjobke.de>
* follow up to nextcloud/server#3151 Signed-off-by: Morris Jobke <hey@morrisjobke.de>
* follow up to nextcloud/server#3151 Signed-off-by: Morris Jobke <hey@morrisjobke.de>
* follow up to #3151 Signed-off-by: Morris Jobke <hey@morrisjobke.de>
|
Looks really nice now: |
|
Yup, damn nice and consistent :) good work all around! Now the only place missing icons is Help :) |
5 participants
Before & after:
This brings it in line with the other apps like Files, Activity, Mail, etc. :)
Please review @nextcloud/designers
A possible future improvement would be to instead of hardcoding the icons via the settings.css to get them from the app icon itself. cc @schiessle @nickvergessen @juliushaertl do you have an idea about the feasibility of that?