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

Remove jQuery #425

Merged
merged 4 commits into from
May 14, 2020
Merged

Remove jQuery #425

merged 4 commits into from
May 14, 2020

Conversation

ablwr
Copy link
Member

@ablwr ablwr commented May 13, 2020

We were using the jQuery library for a few things, but I replaced these things with plain ol' Javascript.

I did some testing here, but it could definitely use more thorough testing to make sure I cleaned it up right! Pushing this now just so I can move on to other tasks today. ;) But will rush back to fix anything that didn't successfully make the transition.

@ablwr ablwr requested review from privatezero and kfrn May 13, 2020 13:36
@@ -2864,4 +2863,5 @@ <h3>*****Longer title*****</h3>
</footer>
</div><!-- ends "grid" -->
</body>
<script src="js/js.js"></script>
Copy link
Member

Choose a reason for hiding this comment

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

Are you sure this should not remain inside the <head> rather than at the <html> level?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh thanks, let me move it up! Or it will be invalid html.

Copy link
Member

@privatezero privatezero left a comment

Choose a reason for hiding this comment

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

Just tested - seems to perform as anticipated on my end

Copy link
Member

@kfrn kfrn left a comment

Choose a reason for hiding this comment

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

I won't have the chance to test until the evening, but LGTM!

js/js.js Outdated
}

// add hash URL when recipe is opened
let recipes = document.querySelectorAll('label[class="recipe"]')
Copy link
Member

Choose a reason for hiding this comment

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

(minor) recipes (and some of the other let variables) can be consts, no?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes! thanks @kfrn -- clearly was just being lazy here

@ablwr ablwr merged commit 3984c8a into gh-pages May 14, 2020
@ablwr ablwr deleted the rm-jquery branch May 14, 2020 12:24
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.

4 participants