Skip to content

Conversation

@pixelipo
Copy link
Contributor

@pixelipo pixelipo commented Jun 16, 2017

There's no reason for not including it and it is needed for Deck app in nextcloud/deck#189

@nextcloud/designers @juliushaertl

Signed-off-by: Marin Treselj marin@pixelipo.com

There's no reason for not including it and it is needed for Deck app.

Signed-off-by: Marin Treselj <marin@pixelipo.com>
@pixelipo pixelipo added 3. to review Waiting for reviews design Design, UI, UX, etc. labels Jun 16, 2017
Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

We need to keep the viewbox attribute though, so all icon resizing in the apps menu and app management works on Firefox, right? cc @MorrisJobke

@pixelipo
Copy link
Contributor Author

@jancborchardt then what about core/img/places/files-dark.svg ?

@codecov
Copy link

codecov bot commented Jun 16, 2017

Codecov Report

Merging #5447 into master will decrease coverage by <.01%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master    #5447      +/-   ##
============================================
- Coverage     54.14%   54.13%   -0.01%     
  Complexity    22329    22329              
============================================
  Files          1380     1380              
  Lines         85544    85544              
  Branches       1329     1329              
============================================
- Hits          46314    46311       -3     
- Misses        39230    39233       +3
Impacted Files Coverage Δ Complexity Δ
core/js/js.js 61.27% <0%> (-0.56%) 0% <0%> (ø)
apps/comments/lib/EventHandler.php 87.5% <0%> (+8.33%) 7% <0%> (ø) ⬇️

@MorrisJobke
Copy link
Member

We need to keep the viewbox attribute though, so all icon resizing in the apps menu and app management works on Firefox, right? cc @MorrisJobke

Correct

@jancborchardt then what about core/img/places/files-dark.svg ?

Please add it there as well. We had weird rendering issues in firefox, especially if the view box is not there and the path is defined in a bigger scope, then firefox misrendered it

Also removed some unneeded style declarations.

Signed-off-by: Marin Treselj <marin@pixelipo.com>
@pixelipo
Copy link
Contributor Author

Ok, now all the places icons (apart from home.svg - that is in #5229 ) have viewport. I've also cleaned up some extra styles.

@juliusknorr
Copy link
Member

We should also introduce a class icon-calendar like there is one for icon-calendar-dark already.

Signed-off-by: Marin Treselj <marin@pixelipo.com>
@juliusknorr juliusknorr added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Jun 19, 2017
@jancborchardt
Copy link
Member

Failures unrelated, merging. :)

@jancborchardt jancborchardt merged commit a5ad6de into master Jun 19, 2017
@jancborchardt jancborchardt deleted the calendar-icon-white branch June 19, 2017 11:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4. to release Ready to be released and/or waiting for tests to finish design Design, UI, UX, etc.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants