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 sphinxext-opengraph #118

Merged
merged 6 commits into from
Jul 22, 2021
Merged

Add sphinxext-opengraph #118

merged 6 commits into from
Jul 22, 2021

Conversation

astrojuanlu
Copy link
Contributor

This should improve the appearance of our links when shared in social media. Close #108.

@astrojuanlu astrojuanlu marked this pull request as draft July 22, 2021 07:56
@astrojuanlu
Copy link
Contributor Author

The generated image link is wrong: background-image: url("https://readthedocs-blog--118.org.readthedocs.build../_images/rebuild.png"); will open an issue upstream about this.

@astrojuanlu
Copy link
Contributor Author

@astrojuanlu
Copy link
Contributor Author

Some caveats:

@astrojuanlu astrojuanlu requested review from ericholscher, nienn and a team July 22, 2021 08:18
@astrojuanlu astrojuanlu marked this pull request as ready for review July 22, 2021 08:18
@humitos
Copy link
Member

humitos commented Jul 22, 2021

but it doesn't quite fit the aspect ratio of media shares

I don't like how it looks 😄

What about using just the logo, https://docs.ops.verbthenouns.com/projects/guidelines/_downloads/ed58d5ce6820efcfb0aa0146ee561409/logo-dark.png

Another option is to update the same image that you are using but with a bigger height (blank) so it scales properly.

@astrojuanlu
Copy link
Contributor Author

I don't like how it looks 😄

Yeah, me neither 😂 testing your proposal

@astrojuanlu
Copy link
Contributor Author

Better now (at least the text is not cut)

Screenshot from 2021-07-22 10-42-48

For anything more sophisticated than this, I'd need to refresh my GNU I.M.P. abilities :)

@nienn
Copy link
Contributor

nienn commented Jul 22, 2021

It's greatly improved. Although I still don't like the look of the oversized logo 😄

Based on this article the best image size should be 1200 x 630.

I'd need to refresh my GNU I.M.P. abilities :)

I can alter the image if there's an option to upload it somewhere.

@astrojuanlu
Copy link
Contributor Author

Super insightful article, thanks @nienn !

I can alter the image if there's an option to upload it somewhere.

In the absence of a better option, I guess we can upload it to the blog itself?

@humitos
Copy link
Member

humitos commented Jul 22, 2021

In the absence of a better option, I guess we can upload it to the blog itself?

Sounds good to me!

@astrojuanlu
Copy link
Contributor Author

@nienn sent me a modified version of the logo, modified from the original SVG for a more appropriate aspect ratio (thanks!). However, as I added it as part of this PR, I won't be able to test it until it's merged.

@nienn
Copy link
Contributor

nienn commented Jul 22, 2021

test

This is NOT a real test on opengraph.xyz. It's just a montage with a preview of what it should look like.

The logo was altered from the official version by spreading the text over 3 lines and changing it's proportion relatively to the icon.

@astrojuanlu
Copy link
Contributor Author

It looks great! When someone submits an accepting ✔️ 🟢 review I will merge the pull request, and we can make further adjustments thereafter

@ericholscher
Copy link
Member

ericholscher commented Jul 22, 2021

@nienn Looks great -- I'll let @agjohnson chime in as current owner of the brand/logo, but I'm guessing that might shift to you here before long ✨

Our official logos are here: https://read-the-docs-guidelines.readthedocs-hosted.com/ -- but we don't have one in that aspect ratio.

@humitos humitos requested a review from agjohnson July 22, 2021 17:09
@agjohnson
Copy link
Contributor

agjohnson commented Jul 22, 2021

Looks good! The wordmark doesn't lend great to being split up, with the italicized the dangling in the middle, but I don't think that's a huge issue given the use case here.

Two options that would preserve the wordmark/logo for this format would be logo or wordmark only, but in the correct ratio, or logo in the background (probably fade out a good deal) and the main, single line wordmark centered overlaying that.

All seem like good options.

@agjohnson
Copy link
Contributor

Also, this image format would be great to include in guidelines repo.

@nienn
Copy link
Contributor

nienn commented Jul 22, 2021

Moving the logo discussion over here: readthedocs/guidelines#1

So it won't keep blocking this PR!

@astrojuanlu
Copy link
Contributor Author

I get from the discussion that this is a good start, and that there will be possibly more iterations on readthedocs/guidelines#1, so I'll go ahead and merge this, so we can start testing how it looks like in real life. Thanks everyone!

@astrojuanlu astrojuanlu merged commit 7a72d4e into main Jul 22, 2021
@astrojuanlu astrojuanlu deleted the add-opengraph branch July 22, 2021 19:57
@astrojuanlu
Copy link
Contributor Author

Just tested it, it works 💯

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.

Adopt sphinxext-opengraph
5 participants