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

Integrate version compare API info into footer_html endpoint #1499

Merged
merged 9 commits into from
Jul 30, 2015

Conversation

gregmuellegger
Copy link
Contributor

This integrates the data provided by /api/v1/version/<project>/highest/<version> into the /api/v2/footer_html/ response. We can use this to avoid one API request per doc page view. The API v1 version endpoint is still in place for backwards compatibility.

I also refactored the doc-embed JavaScript into multiple files for easier maintainability.

Fixes #1463.

@ericholscher
Copy link
Member

Big +1 on making our JS more managable.

}


function setupBookmarkCSRFToken() {
Copy link
Member

Choose a reason for hiding this comment

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

We can probably kill this for now -- we have killed the Bookmarking feature (it was half done, and probably not the most reasonable from a product perspective). However, this code does do some interesting things that we'll likely need to do when we actually look seriously at doing CNAME-supported javascript clients.

@ericholscher
Copy link
Member

Looks like a solid refactor. 👍 -- Do we need to include any changes in the gulp configs or anything along those lines, or will it automagically just work?

Edit: I see we're just making the existing readthedocs-doc-embed.js more modular, makes sense.

ericholscher added a commit that referenced this pull request Jul 30, 2015
Integrate version compare API info into footer_html endpoint
@ericholscher ericholscher merged commit cdb0e52 into master Jul 30, 2015
@ericholscher
Copy link
Member

🎆

ericholscher added a commit that referenced this pull request Jul 30, 2015
This reverts commit cdb0e52, reversing
changes made to 26abb6b.
@ericholscher
Copy link
Member

This broke when I tested it, so I've reverted it for now.

It was depending on READTHEDOCS_DATA always being defined before the javascript was included, which is not the case in all themes.

ericholscher added a commit that referenced this pull request Jul 30, 2015
…js""

This reverts commit 13a8162.

Conflicts:
	readthedocs/core/static-src/core/js/readthedocs-doc-embed.js
	readthedocs/core/static/core/js/readthedocs-doc-embed.js
ericholscher added a commit that referenced this pull request Jul 30, 2015
…js""

This reverts commit 13a8162.

Conflicts:
	readthedocs/core/static-src/core/js/readthedocs-doc-embed.js
	readthedocs/core/static/core/js/readthedocs-doc-embed.js
@gregmuellegger
Copy link
Contributor Author

Sorry mate! 😿 I have addressed the raised issues in #1507

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Improvement Minor improvement to code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants