Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added white download icons #23891

Merged
merged 1 commit into from
Apr 10, 2016
Merged

Added white download icons #23891

merged 1 commit into from
Apr 10, 2016

Conversation

skjnldsv
Copy link
Contributor

If we got white upload icons, we could really use some white download ones.
I'm in need of some for the contacts app, and I think it should be added in core for future possible uses.

Tell me if you think this is a bad idea :)
@owncloud/designers @Henni

@@ -86,6 +86,10 @@
background-image: url('../img/actions/download.svg');
}

Copy link
Member

Choose a reason for hiding this comment

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

Cut this line break, so it belongs to the icon-download. Otherwise nice!

@jancborchardt
Copy link
Member

Nice! Looks like both are correctly minified/compressed, right?

@skjnldsv
Copy link
Contributor Author

Well, i used the download icons black, revert the png and just added the fill parameter to the svg (like the upload-icon-white) Don't know if this could me more compressed :)

@jancborchardt Line break removed.

@jancborchardt
Copy link
Member

Yup, looks good! 👍

Just squash the commits into one (the first, with the good commit message). :)

You can also in the future just amend your commits with:

git add your files
git commit --amend (keep the same commit message)
git push -f origin branch-name

@skjnldsv
Copy link
Contributor Author

Without git rebase?

@skjnldsv skjnldsv force-pushed the add-download-icon-white branch from 6fd37e9 to 408bdd6 Compare April 10, 2016 11:54
@jancborchardt
Copy link
Member

Yeah, rebase is optional but different, just for updating it to current master. Amending and force pushing is for using the same commit in the current branch and overwriting it on the remote to not create two commits.

@skjnldsv
Copy link
Contributor Author

Nice, I was always using a rebase HEAD to merge multiple commits. I only used ammend to correct an unpushed commit title.
Thanks!

@irgendwie
Copy link

LGTM 👍

@skjnldsv
Copy link
Contributor Author

let get this merged?

@skjnldsv skjnldsv added this to the 9.1-current milestone Apr 10, 2016
@skjnldsv
Copy link
Contributor Author

@jancborchardt should we merge into 9.0.2 too?

@DeepDiver1975
Copy link
Member

👍

@lock
Copy link

lock bot commented Aug 6, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants