Remove jQuery in embedded_record.js #4962
Conversation
demiankatz
left a comment
demiankatz
left a comment
There was a problem hiding this comment.
Thanks, @ThoWagen, this looks good overall, but see below for one minor suggestion and one question. I can do more hands-on testing when I have more time; this is just a visual inspection of code as a first pass!
| let id = idElement.value; | ||
| let source = sourceElement.value; | ||
| if (tab.parentElement.classList.contains('noajax')) { | ||
| let link = tab.querySelector('a'); |
There was a problem hiding this comment.
Have you tested that this works for both the tab and accordion cases? It would be nice if this can be simplified, but I'm curious why the separate cases from the jQuery version are no longer needed. I can take the time to do hands-on testing if it would help!
There was a problem hiding this comment.
No, I missed that because the tab version will not be needed in #4901 anymore. But I fixed it here for now.
demiankatz
added
the
architecture
demiankatz
left a comment
demiankatz
left a comment
There was a problem hiding this comment.
Thanks, @ThoWagen, this looks good to me now, and all tests (both automated and manual) are passing for me.
2 participants
Part of #4901