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

Mention "Hosted on Read the Docs" in the footer #1616

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

humitos
Copy link
Member

@humitos humitos commented Oct 8, 2024

Screenshot_2024-10-08_15-31-26

Closes #1587

@agjohnson
Copy link
Collaborator

Not sure if I understand the need for this link from the underlying issue, but it feels a bit redundant or maybe even spammy. Is there a reason we need a second link to us?

@humitos
Copy link
Member Author

humitos commented Oct 8, 2024

We don't need a second link. I removed the old one in 14d07f9

@agjohnson
Copy link
Collaborator

To clarify, it's not just the link, it's the text that feels redundant/spammy. This will still render as:

Built with Sphinx using a theme provided by Read the Docs. Hosted by Read the Docs.

This still feels redundant, maybe spammy -- it doesn't feel to me like we need our name in there twice on every page.

Built with Sphinx and hosted on Read the Docs.

That seems fine.

I'm also fine not mentioning hosting. I think it's a surprise to users when there is conditional content like this -- locally this string isn't there but our builds produce different output.

{% endif %}

{% if READTHEDOCS %}
Hosted on <a href="https://about.readthedocs.com/">Read the Docs</a>.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should probably be wrapped by show_sphinx or a specific configuration option. If the user selected show_sphinx = False, they will probably not like that we injected different content in the same space.

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.

Split flyout integration into different selectors
2 participants