Embedded js: remove some dependency from jquery #9508
Conversation
| @@ -275,23 +296,27 @@ function attach_elastic_search_query_mkdocs(data) { | |||
| var results = data.results || []; | |||
|
|
|||
| if (results.length) { | |||
| var searchResults = $('#mkdocs-search-results'); | |||
| searchResults.empty(); | |||
| let searchResults = document.getElementById('mkdocs-search-results'); | |||
There was a problem hiding this comment.
We could also just kill the mkdocs integration, isn't currently being used (only on my test project).
There was a problem hiding this comment.
I'm 👍🏼 on removing things we are not using 😄
There was a problem hiding this comment.
Perhaps this is something that we instead address with generic version of our search integration. I'm 👍 in both directions, if we aren't using the code it shouldn't hurt to keep it around either, but also happy to see unused code disappeared.
There was a problem hiding this comment.
I think we should remove it, and work towards shipping our livesearch as a generic JS library. The less code we have to maintain the better. 👍
| @@ -275,23 +296,27 @@ function attach_elastic_search_query_mkdocs(data) { | |||
| var results = data.results || []; | |||
|
|
|||
| if (results.length) { | |||
| var searchResults = $('#mkdocs-search-results'); | |||
| searchResults.empty(); | |||
| let searchResults = document.getElementById('mkdocs-search-results'); | |||
There was a problem hiding this comment.
Perhaps this is something that we instead address with generic version of our search integration. I'm 👍 in both directions, if we aren't using the code it shouldn't hurt to keep it around either, but also happy to see unused code disappeared.
There was a problem hiding this comment.
Looking at the previous comments, I think this is good to ship. @agjohnson if you want to give it another look, go ahead, but it seems like you already reviewed it and it's 👍 with the review updates.
Also agree that we need to QA this pretty heavily, given the lack of testing 🙃
| @@ -30,21 +30,6 @@ function injectFooter(data) { | |||
| } | |||
|
|
|||
|
|
|||
| function setupBookmarkCSRFToken() { | |||
| @@ -275,23 +296,27 @@ function attach_elastic_search_query_mkdocs(data) { | |||
| var results = data.results || []; | |||
|
|
|||
| if (results.length) { | |||
| var searchResults = $('#mkdocs-search-results'); | |||
| searchResults.empty(); | |||
| let searchResults = document.getElementById('mkdocs-search-results'); | |||
There was a problem hiding this comment.
I think we should remove it, and work towards shipping our livesearch as a generic JS library. The less code we have to maintain the better. 👍
This needs readthedocs/common#155.
Basically replacing ajax calls with the fetch API, and using the DOM API for the manipulation of elements.
Note: we need to create a rule on cloudflare to not cache the analytics endpoint,
we were previously using a hack for this (passing a random query arg). In the future, we should set this at the view level (no-cache). Already did this for .org and .com.