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

Fix/remove jquery #656

Open
wants to merge 22 commits into
base: develop
Choose a base branch
from
Open

Fix/remove jquery #656

wants to merge 22 commits into from

Conversation

DavidCramer
Copy link
Contributor

No description provided.

Copy link
Member

@pereirinha pereirinha left a comment

Choose a reason for hiding this comment

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

@DavidCramer

I left you a few questions in this PR.

@@ -464,6 +464,9 @@ private function init_taxonomy_manager( $post ) {
}

$out[] = '</div>';
if ( ! function_exists( 'use_block_editor_for_post' ) || ! use_block_editor_for_post( $post ) ) {
wp_enqueue_script( 'term-ordering', $this->media->plugin->dir_url . '/js/classic-editor.js', array(), $this->media->plugin->version, true );
Copy link
Member

Choose a reason for hiding this comment

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

@DavidCramer
Since we are changing the script variable name inside, from TermsOrder to something more abstract like ClassicEditor, perhaps we can find a more abstract slug?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pereirinha - Good idea.

@@ -1,6 +1,6 @@
/* global wpAjax */

const TermsOrder = {
const ClassicEditor = {
Copy link
Member

Choose a reason for hiding this comment

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

@DavidCramer

There are still many references to jQuery in this script.
Can't we remove them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pereirinha Oops, I messed some 🤦 - forgot about the jQuery over the $

@DavidCramer DavidCramer marked this pull request as draft January 28, 2022 09:23
@DavidCramer DavidCramer marked this pull request as ready for review January 30, 2022 08:38
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.

2 participants