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

Addons: integrate with new beta addons flyout #1526

Merged
merged 28 commits into from
Jul 15, 2024
Merged

Conversation

humitos
Copy link
Member

@humitos humitos commented Sep 13, 2023

Initial experimentation to use the CustomEvent triggered by the addons called readthedocs-addons-data-ready event (from readthedocs/addons#64) to build the Read the Docs flyout being integrated into the theme keeping the original look & feel.

How to test it

  1. Build 'n Serve the documentation setting READTHEDOCS variable
READTHEDOCS=True sphinx-autobuild --watch sphinx_rtd_theme/versions.html -a --port 9999 --host 0.0.0.0 docs docs/_build/html
  1. Start the addons development server from humitos/custom-event branch
cd readthedocs/addons
git checkout origin/main
npm run dev
  1. Open the browser at http://localhost:9999/ (note that we are using localhost here)

Example

Screenshot_2023-09-13_18-04-07

Note

I added these lines in versions.html

<meta name="readthedocs-resolver-filename" content="/" />
<script type="text/javascript" src="http://localhost:8000/readthedocs-addons.js"></script>

and also hardcoded the URL to hit in the readthedocs-config.js addons file as

  url = "http://docs.devthedocs.org/_/addons/?project-slug=docs&version-slug=latest&client-version=0.12.0&api-version=1";

and used Firefox CORS Addon to allow CORS with the development instance


Initial experimentation to use the `CustomEvent` triggered by the addons called
`readthedocsdataready` event (from
readthedocs/addons#64) to build the Read the Docs
flyout being integrated into the theme keeping the original look & feel.

* Related: readthedocs/addons#64
* Closes #1523
This is because we are not executing the Read the Docs magic that modifies the
`conf.py` file on the fly :/
@agjohnson
Copy link
Collaborator

Noted on a call today, we should start trying out these changes for some of our own projects to start. This would serve as a working example for other authors, and gives us something to test against.

sphinx_rtd_theme/versions.html Outdated Show resolved Hide resolved
sphinx_rtd_theme/versions.html Outdated Show resolved Hide resolved
@humitos
Copy link
Member Author

humitos commented Apr 17, 2024

Not sure how complete this is, I didn't test this locally. But this seems on the right path

It's great knowing that we are going in the right direction. The PR is not finished or ready to merge yet, but I wanted to have a review at this stage to know if we are aligned here and it seems we are 👍🏼

@humitos
Copy link
Member Author

humitos commented Apr 17, 2024

With the latest changes, it looks great to me! I think we are ready to merge readthedocs/readthedocs.org#11279 so we can start testing it out in our documentation.

Screenshot_2024-04-17_13-43-59

@agjohnson
Copy link
Collaborator

Yeah lets merge the docs PR 👍 This seems close enough to start testing

humitos added a commit to readthedocs/readthedocs.org that referenced this pull request Apr 18, 2024
…11279)

* Docs: use the `sphinx-rtd-theme` with support for addons integration

Installs a version of our theme from a pull request that has support for the
Addons integration using the `CustomEvent`.

Related: readthedocs/sphinx_rtd_theme#1526
Requires: #11205

* Disable fail on warning temporary

* Delete doc-diff since it's included in addons now
Copy link
Collaborator

@agjohnson agjohnson left a comment

Choose a reason for hiding this comment

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

We discussed next steps here today, and I think to consider an alpha/beta release of this, we should use the existing patterns for JS/CSS instead. Besides that, this all seems fairly close.

We talked about testing this with more internal projects, which is a good step as well. I don't see to much changing besides finishing the JS/CSS.

sphinx_rtd_theme/versions.html Outdated Show resolved Hide resolved
sphinx_rtd_theme/versions.html Outdated Show resolved Hide resolved
sphinx_rtd_theme/versions.html Outdated Show resolved Hide resolved
@humitos humitos requested a review from agjohnson June 25, 2024 14:59
@humitos
Copy link
Member Author

humitos commented Jun 25, 2024

I made the requested changes 👍🏼 . It seems everything keeps working fine: https://docs.readthedocs.io/en/latest/

@humitos
Copy link
Member Author

humitos commented Jun 25, 2024

Failing tests are unrelated with the changes on this PR. It seems the latest Sphinx/docutils version changed something in the HTML structure and we will need to update our test case.

@humitos
Copy link
Member Author

humitos commented Jul 11, 2024

@agjohnson can you take a look at this PR? I'd like to move forward with an alpha release, so we can start testing it with some particular users.

Copy link
Collaborator

@agjohnson agjohnson left a comment

Choose a reason for hiding this comment

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

I haven't had time to do too deep a review here, but this looks like it should be close enough. 👍

We'll likely need to use the beta period to debug issues with this a bit, as it can be hard to code defensively in string interpolations.

@humitos
Copy link
Member Author

humitos commented Jul 15, 2024

Cool! I'm merging this PR now. We can coordinate the release of an alpha/beta version after that 👍🏼

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.

Integrate the new addons flyout into the theme
2 participants