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

Replace icon with logo with text #87

Merged
merged 3 commits into from Aug 20, 2016
Merged

Replace icon with logo with text #87

merged 3 commits into from Aug 20, 2016

Conversation

rugk
Copy link
Member

@rugk rugk commented Aug 20, 2016

  • I agree to release the changes in this PR under the Zlib/libpng license as shown in the LICENSE file.

Changes

  • Remove icon.svg with logo.svg
  • Change templates to use new logo including correct size (225px width)

ToDo

  • The icon does not look so nice in such a small version (thick border), needs to be updated

@rugk rugk added this to the first release after name switch milestone Aug 20, 2016
@rugk rugk changed the title [WIP] Replace icon with logo with text Replace icon with logo with text Aug 20, 2016
@elrido
Copy link
Contributor

elrido commented Aug 20, 2016

There seem to be some conflicts and could you also replace the icon in the main README?

@rugk
Copy link
Member Author

rugk commented Aug 20, 2016

It is already replaced in the Readme.

I've also fixed the conflicts.

@elrido elrido merged commit 0570470 into master Aug 20, 2016
@rugk rugk deleted the newicon branch August 20, 2016 17:10
@elrido
Copy link
Contributor

elrido commented Aug 20, 2016

Sorry, we probably were posting simultaneously.

@elrido
Copy link
Contributor

elrido commented Aug 21, 2016

I have to say that the logo with text in the template looks awful (in browsers) and is way too large. Couldn't we just keep the icon and make it a bit smaller (38 pixels instead of 42)? On the README.md the logo with text looks OK, so we can keep it there.

Update: I think what happens is that FF renders it in the size as stated in the metadata of the logo (500 pixels) and then scales down the rendered pixel images. So adjusting the size to the target size in the settings of the image should help a lot. Still id prefer a logo with just the icon in the top left. Or at least make the font much less bold or a lot less black (maybe grey).

@rugk
Copy link
Member Author

rugk commented Aug 21, 2016

is way too large.

Well... previously where we had no icon it was similar.

We can of course revert this PR and use the icon.
It just matters what you want: only the icon or do you want to have the text? If so we could adjust the size again and make it less intrusive, of course.

BTW: On mobile view we should probably always use the icon only.

@elrido
Copy link
Contributor

elrido commented Aug 21, 2016

Ok, how about we keep this current logo as the 500px version for README, but create a second one without the text and in size 38px (just the icon with that size setting in inkscape). We then simply keep the "PrivateBin" as text next too it. That will work in the bootstrap, darkstrap and page templates.

We can then still review the mobile version to see if we need to remove the text or not in it. There should be enough space even with just 360 pixels width, since the menubar disappears into the hamburger menu in that mode.

@rugk rugk assigned rugk and unassigned elrido Aug 21, 2016
@rugk
Copy link
Member Author

rugk commented Aug 21, 2016

The height should be 42px, 38 is a bit too small, but yes we can create one here.

So I'll revert this PR and add the icon again.

@elrido
Copy link
Contributor

elrido commented Aug 21, 2016

No need to revert it, just create a new one.

42px is to high, the hexagons tip almost touches the viewport. For good design there should always be a space of at least 3 - 5 pixels to surrounding elements (the text part of the logo next to it can be tighter, it is part of the logo element group).

rugk added a commit that referenced this pull request Aug 21, 2016
@rugk rugk mentioned this pull request Aug 21, 2016
1 task
@rugk rugk restored the newicon branch December 15, 2016 17:55
@rugk rugk deleted the newicon branch December 15, 2016 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants