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

Add logo to README, and make intro it look a little nicer #2003

Merged
merged 2 commits into from Feb 19, 2018

Conversation

astorije
Copy link
Member

@astorije astorije commented Jan 25, 2018

View the result here

Ideas welcome :)

@astorije astorije added the Type: Documentation Lack of documentation, improvement suggestion, or PRs that address these. label Jan 25, 2018
@astorije astorije added this to the 3.0.0 milestone Jan 25, 2018
@astorije
Copy link
Member Author

@xPaw, I really gave a chance to the for-the-badge style, but couldn't make them look nice. They are way too big, and when reducing size, the text gets rather hard to read. Also they have margin issues, so they look wonky next/on top of each other. Too early I guess 😅

Also I really wanted to give a shot at Travis CI Build Status / AppVeyor Build Status but I think it doesn't look good (well, the Travis one, at least), and it would be weird if only those 2 had logo. We can use Unicode but meh.

@xPaw
Copy link
Member

xPaw commented Jan 25, 2018

Damn that's a lot of badges. I would prefer if they fit on 1 or 2 lines at most.

TL_Grey&Yellow_Vertical_logotype_White_Bg.svg

Don't think it's a good idea to put ampersands in URLs like that. I think all the logos should be added to an assets/logos folder, so they all can be seen. And then the ones client uses copied and optimized (with svgo) separately.

@astorije
Copy link
Member Author

Damn that's a lot of badges. I would prefer if they fit on 1 or 2 lines at most.

It's "only" 3 badges more than what we have. You should see it as 2 lines though, the first line is just the freenode channel, to emphasize on it. Overall, it makes for something rather balanced and nice, I think, not overwhelmed by badges:

screen shot 2018-01-26 at 01 06 26

The 3 logos I added are the Node.js version supported, MIT license, and total downloads. Of course, none of those are mandatory, I just thought they were informative enough to go there. Node version, for example, especially since we're changing it in v3... License is meh, downloads is a nice pat in the back, but not vital I agree :)

With the current badges, on a single line (again, not counting the freenode channel as its own line), it gives:

screen shot 2018-01-26 at 01 11 15

With Node version as only extra badge:

screen shot 2018-01-26 at 01 12 41

What do you think? I'm fine with any of those 3, and don't want to overthink this lol, just make it look nice :)

Don't think it's a good idea to put ampersands in URLs like that. I think all the logos should be added to an assets/logos folder, so they all can be seen.

Oh yeah absolutely. I should have added a todo item about this, because it's not what I want to ship, I was just doing the overall shape at that point.

@MaxLeiter
Copy link
Member

I like the way you have it - does feel balanced and clean

@astorije
Copy link
Member Author

@xPaw, we need to decide which file(s) we want to add to this repo. I'm adding everything as provided to the website repo, but this one should only contain a curated version, i.e. not the .ai files, not the brand guidelines, etc.
And it would be best if we can upload only SVG unless cases where PNG is required (like for manifest or iOS or whatnot).

I figured I didn't have to do this just now, so finished the README first, but happy to hear thoughts.

@xPaw
Copy link
Member

xPaw commented Jan 26, 2018

@astorije I think second screenshot is the best. Having too many badges (especially static ones) is just too noisy. We could probably add downloads badge into that (instead of node version). And change green color to something else so it doesn't collide with builds.

Copy link
Member

@xPaw xPaw left a comment

Choose a reason for hiding this comment

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

Image needs renaming though.

@astorije
Copy link
Member Author

@xPaw, changed, see result below. I'm actually changing my mind on downloads, since over time the npm might not be the prime way to install / download The Lounge: it will be better to do it via your OS package manager.

screen shot 2018-01-27 at 16 54 03

@xPaw
Copy link
Member

xPaw commented Jan 27, 2018

@astorije That's true, although that still gives us some popularity metric.

@astorije
Copy link
Member Author

Ahah yeah, nevermind: https://img.shields.io/github/downloads/thelounge/lounge/total.svg (not documented at the moment though).
Ah, how I would love automated deb builds...

@astorije astorije force-pushed the astorije/logo-readme branch 5 times, most recently from b8b375b to b637f50 Compare January 28, 2018 06:53
@astorije
Copy link
Member Author

Updated the name of the image, and optimized it with svgo --multipass:

logo-vertical-transparent-bg.svg:
Done in 81 ms!
3.952 KiB - 33.6% = 2.624 KiB

Any suggestion for the tag line? :)

@MaxLeiter
Copy link
Member

MaxLeiter commented Jan 28, 2018

Not sure how big of a deal this is, but transparent background doesn’t seem to work right on mobile

@astorije
Copy link
Member Author

😩
@xPaw should we switch to PNG with transparent background?

@xPaw
Copy link
Member

xPaw commented Feb 1, 2018

@astorije perhaps background-color:transparent can be specified in the svg?

EDIT: It's github's fault. They set background-color on img, and seemingly incorrect on mobile.

@astorije
Copy link
Member Author

astorije commented Feb 1, 2018

EDIT: It's github's fault. They set background-color on img, and seemingly incorrect on mobile.

Yep...
This is on Chai's repo with a transparent-background PNG:

screen shot 2018-02-01 at 18 55 29

Same issue.

So this should stay as is, and I'll send a ticket to their support, linking to this issue.
EDIT: Done.

<img
width="300"
alt="The Lounge"
src="client/img/logo-vertical-transparent-bg.svg">
Copy link
Member

Choose a reason for hiding this comment

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

This could link to image from website repository.

Copy link
Member Author

Choose a reason for hiding this comment

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

I hesitated between link to the main website or nothing, and went for nothing because it's just too random :) Plus we already have a link to the website underneath. But link to the website repo, I'm not a fan, it's plain weird

Copy link
Member

Choose a reason for hiding this comment

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

I meant image link

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, right, but it's already a link to the image (within this repo) so 🤷‍♂️

Copy link
Member

Choose a reason for hiding this comment

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

We wouldn't have to track the logo separately in this repo, just like we don't do it for the screenshot.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh wow, I realize I totally misread what you meant from the beginning. I get it now.

I'm going to use that same image file in the app itself, so I'd have to add it anyway.

README.md Outdated
@@ -18,10 +66,6 @@ Modern web IRC client designed for self-hosting.

To learn more about configuration, usage and features of The Lounge, take a look at [the website](https://thelounge.github.io).
Copy link
Member

Choose a reason for hiding this comment

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

Change the url here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Documentation Lack of documentation, improvement suggestion, or PRs that address these.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants