Skip to content

add fullscreen icon - #1803

Merged
MorrisJobke merged 1 commit into
masterfrom
fullscreen-icon
Oct 19, 2016
Merged

add fullscreen icon#1803
MorrisJobke merged 1 commit into
masterfrom
fullscreen-icon

Conversation

@jancborchardt

Copy link
Copy Markdown
Member

Useful for Gallery, Spreed and other things. We should also backport it to the stable branches. cc @karlitschek

Please review @nextcloud/designers

@jancborchardtjancborchardt added this to the Nextcloud 11.0 milestone Oct 19, 2016
@mention-bot

Copy link
Copy Markdown

@jancborchardt, thanks for your PR! By analyzing the history of the files in this pull request, we identified @skjnldsv, @LukasReschke and @nickvergessen to be potential reviewers.

@LukasReschke

Copy link
Copy Markdown
Member

LGTM! 🚀

@skjnldsv

Copy link
Copy Markdown
Member

To the moon!
👍

Comment threadcore/img/actions/fullscreen.svg Outdated
@@ -0,0 +1,4 @@
<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<svg xmlns="http://www.w3.org/2000/svg" height="16" width="16" version="1.1">

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

missing viewbox, so firefox does not rage quit?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

yes, please viewBox="0 0 16 16"

@karlitschek

Copy link
Copy Markdown
Member

great idea 👍

@MorrisJobkeMorrisJobke added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Oct 19, 2016
@rullzer

Copy link
Copy Markdown
Member

Why backport? This is introducing something new.

@MariusBluem

Copy link
Copy Markdown
Member

Because Spreed, Gallery and maybe other apps will depend on it 😁 @rullzer

Signed-off-by: Jan-Christoph Borchardt <hey@jancborchardt.net>
@jancborchardt

Copy link
Copy Markdown
MemberAuthor

Yup, what @MariusBluem said. If we use it in these apps, they need to stay backwards-compatible.

@rullzer

Copy link
Copy Markdown
Member

Well they should not depend on stuff that wasn't there. Ship their own icons. Or say they only support NC11.

But I'm not having the whole backport discussion again.

@codecov-io

Copy link
Copy Markdown

Current coverage is 57.35% (diff: 100%)

Merging #1803 into master will increase coverage by <.01%

@@ master #1803 diff @@
==========================================
Files 1075 1075 Lines 61313 61315 +2 Methods 6865 6865 Messages 0 0 Branches 0 0 ==========================================
+ Hits 35163 35166 +3 + Misses 26150 26149 -1 
Partials 0 0 

Sunburst

Powered by Codecov. Last update 58d53eb...c52ccbd

@MorrisJobkeMorrisJobke added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Oct 19, 2016
@MorrisJobke

Copy link
Copy Markdown
Member

👍

@MorrisJobke
MorrisJobke merged commit ba0cab7 into masterOct 19, 2016
@MorrisJobke
MorrisJobke deleted the fullscreen-icon branch October 19, 2016 22:03
@jancborchardt

Copy link
Copy Markdown
MemberAuthor

stable10 #1818, stable9 #1819

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to reviewWaiting for reviewsdesignDesign, UI, UX, etc.enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants

@jancborchardt@mention-bot@LukasReschke@skjnldsv@karlitschek@rullzer@MariusBluem@codecov-io@MorrisJobke@nickvergessen