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 a favicon for the docs #437

Merged
merged 1 commit into from
Aug 16, 2023
Merged

Add a favicon for the docs #437

merged 1 commit into from
Aug 16, 2023

Conversation

mgmacias95
Copy link
Contributor

Issue number of the reported bug or feature request: None

Describe your changes
Add a favicon for the documentation website.

Testing performed
I deployed a github pages website on my own fork: https://mgmacias95.github.io/memray/

@godlygeek
Copy link
Contributor

This is a good idea, but I think we should be able to get a sharper image.

@pablogsal you've got an SVG of the logo kicking around somewhere, right? Cropping that to a square and then scaling down might get us something sharper, though we probably will need to crop most of the tail out - or make it wrap under the body, maybe, if we're willing to actually do some image editing...

@mgmacias95
Copy link
Contributor Author

I have PNG versions of the favicon, maybe those look slightly shaper:
android-chrome-512x512
apple-touch-icon
favicon-32x32

I tried multiple options but the best looking one was cropping the tail. I'm happy to do some image editing if you have other ideas.

@pablogsal
Copy link
Member

@pablogsal you've got an SVG of the logo kicking around somewhere, right?

Not for memray. I only have the png image

@godlygeek
Copy link
Contributor

godlygeek commented Aug 15, 2023

OK, that's too bad. An SVG would've been really nice for trying to downscale gracefully. Ah well.

@mgmacias95 I'd like to make a different suggestion here, then. Would you mind trying to make a favicon that's just the head of the snake, and let us compare that to what you've got so far? I've mocked up this as an example of what I'm picturing:

idea 1

but I'm not actually good with photoshop, and I'm sure someone competent could greatly improve upon that. I'm thinking that focusing only on the head would let us have enough resolution at 32x32 pixels for the tongue to be visible, and I think the head is probably the most recognizable part of the logo for people, so it makes sense to spotlight that part. It might even be better to leave off even more of the body, and do something like:

idea 2

I like that even more than my first attempt (which looks a little too Loch Ness Monster in comparison 😆)

Would you mind taking this idea and running with it a bit?

@mgmacias95
Copy link
Contributor Author

Hello @godlygeek,

I made a proposal based on your suggestions. Let me know if you like it to integrate it on the PR :).

memray

Cheers,
Marta

@godlygeek
Copy link
Contributor

That looks pretty amazing to me! How does it look when scaled down to favicon size? Do we retain enough detail for it to be recognizable?

@mgmacias95
Copy link
Contributor Author

Hello @godlygeek,

I'm glad you liked it! I updated the website I deployed with the new favicon. Check it out.

Thanks!

@godlygeek
Copy link
Contributor

godlygeek commented Aug 16, 2023

Looks nice to me!

Any objections to the favicon from https://mgmacias95.github.io/memray/ @pablogsal ? If not, I'm happy to squash and land this.

@pablogsal
Copy link
Member

No objections here! Great job @mgmacias95 !

Signed-off-by: Marta Gómez Macías <mgmacias@google.com>
Copy link
Contributor

@godlygeek godlygeek left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for the contribution, @mgmacias95!

@godlygeek godlygeek merged commit 3b29f01 into bloomberg:main Aug 16, 2023
6 checks passed
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

3 participants