Added bell in black (#000000) and grey (#777777)#185
Conversation
michaelstingl
left a comment
michaelstingl
left a comment
There was a problem hiding this comment.
please cleanup the SVG's and follow naming convention from other repos
|
@tomneedham is it okay to add some more image styles? (colors and names inspired from https://github.com/owncloud/core/tree/master/core/img/actions ) |
|
afaik we are controlling icon colors with css on svgs - but maybe I'm mixing up things ..... @felixheidecke |
Codecov Report
@@ Coverage Diff @@
## master #185 +/- ##
=========================================
Coverage 89.78% 89.78%
Complexity 137 137
=========================================
Files 18 18
Lines 548 548
=========================================
Hits 492 492
Misses 56 56
Continue to review full report at Codecov.
|
DeepDiver1975
left a comment
DeepDiver1975
left a comment
There was a problem hiding this comment.
please squash commits - THX
+ Grey bell (#777777) + Black bell (#000000) Make changes over Atom not in Illustrator + Changed grey to #717171 Call it "dark" instead of "black" + To follow naming convention from other repos
|
You're welcome @DeepDiver1975 |
|
Any updates here? @felixheidecke Please answer the last question "afaik we are controlling icon colors with css on svgs - but maybe I'm mixing up things ..... " from @DeepDiver1975 |
|
We are indeed controlling colors inside the SVG itself. So no need to worry. |
|
@felixheidecke could you point to the docs or an example somewhere? |
|
You can see here, that currently svg's have strokes and fills to determine colors. |
|
@felixheidecke what technique do you recommend to manipulate stroke and fill for a theme? CSS? JS? Could you point to an example that illustrates this? |
|
SVG images can only be styled via CSS if you write it INTO the file itself or if you have the XML part in the DOM. Since we don't do the latter but rather reference it in an IMG tag, there is no way of outside CSS styling. Currently, there is no way to have one SVG file and attach styling classes. The solution here is to embed icons in a webfont file and have that be styled via outside CSS. Are you guys interested in that approach? |
|
@felixheidecke If we don't have a magic trick to change SVG colours yet, we need to add additional SVG's. This is the case in many other places, and should be done here too. @DeepDiver1975 Please approve this PR. |
|
@ChrisEdS As a workaround, you'd need to ship a set of icons with every customized theme: https://github.com/owncloud/theme-enterprise/tree/master/settings/img |
|
@DeepDiver1975 Any chance to merge this PR? |
|
This change was not backported to notifications app |
|
Hm. It's not transparent which apps get released from |
We are in the process of unifying this |
😍 |
|
please provide some test steps for QA. it's not clear to me when these icons are supposed to appear |
|
In a lot of themes the white bell is useless (because of missing contrasts), this is an approach to give the user the opportunity to switch to a bell with another color. |
|
@ChrisEdS can you test this with 10.2.0 RC1 then ? or provide detailed instructions how to test this. who is "the user" and how can "a user" make use of this ? do you mean a theme developer ? |
|
A user can be anybody. But indeed he has to develop his theme. I will test this in the 10.2.0 RC1. |
|
thanks. in terms of personas, under "user" we usually understand "a regular ownCloud user" who has no access to the underlying system and also no admin privileges. Even an "ownCloud admin" is only able to administrate users within an ownCloud instance. The "sysadmin" role would be the one who has access to the filesystem and occ. In the case of themes, I'd also call "theme developer" whoever is developing a theme, which might get deployed by the "sysadmin". |
|
You're right about that. I will take that into account in the future. Thank you. |
8 participants
How to test:
When notification bell appears go to the site inspector (Right click -> inspect)
notifications-dark.svg
notifications-grey.svg
...