From c011e998191c3d9f62a460c787ccd2f27e6b49d0 Mon Sep 17 00:00:00 2001 From: Emi Matchu Date: Wed, 13 Mar 2024 21:26:22 -0700 Subject: [PATCH] Fix various JS Turbo issues First one, Turbo reasonably yelled at us in the JS console that we should put its script tag in the `head` rather than the `body`, because it re-executes scripts in the `body` and we don't want to spin up Turbo multiple times! I also removed some scripts that aren't relevant anymore, fixed a bug in `outfits/new.js` where failing to load a donation pet would cause the preview thing to not work when you type (I think this might've already been an issue?), reworked `item_header.js` to just run once in the `head`, and split scripts into `:javascripts` (run once in `head`) vs `:javascripts_body` (run every page load in `body`). --- .../javascripts/closet_hangers/index.js | 10 --- .../javascripts/fundraising/campaigns/show.js | 66 ------------------- app/assets/javascripts/items/item_header.js | 29 ++++---- app/assets/javascripts/outfits/index.js | 3 - app/assets/javascripts/outfits/new.js | 2 +- app/helpers/application_helper.rb | 2 +- app/views/closet_hangers/index.html.haml | 5 +- .../fundraising/campaigns/show.html.haml | 4 -- .../fundraising/donations/show.html.haml | 4 +- app/views/items/_item_header.html.haml | 2 +- app/views/items/show.html.haml | 2 +- app/views/layouts/application.html.haml | 10 ++- app/views/outfits/index.html.haml | 5 -- app/views/outfits/new.html.haml | 5 +- app/views/pets/bulk.html.haml | 5 +- 15 files changed, 38 insertions(+), 116 deletions(-) delete mode 100644 app/assets/javascripts/fundraising/campaigns/show.js delete mode 100644 app/assets/javascripts/outfits/index.js diff --git a/app/assets/javascripts/closet_hangers/index.js b/app/assets/javascripts/closet_hangers/index.js index aea1d0c7..c78019b0 100644 --- a/app/assets/javascripts/closet_hangers/index.js +++ b/app/assets/javascripts/closet_hangers/index.js @@ -747,16 +747,6 @@ } }); - /* - - Hanger list controls - - */ - - $("input[type=submit][data-confirm]").live("click", function (e) { - if (!confirm(this.getAttribute("data-confirm"))) e.preventDefault(); - }); - /* Closet list droppable diff --git a/app/assets/javascripts/fundraising/campaigns/show.js b/app/assets/javascripts/fundraising/campaigns/show.js deleted file mode 100644 index 3d413eb1..00000000 --- a/app/assets/javascripts/fundraising/campaigns/show.js +++ /dev/null @@ -1,66 +0,0 @@ -(function() { - var donationForm = document.getElementById('donation-form'); - - function field(name) { - return donationForm.querySelector( - 'input[name=donation\\[' + name + '\\]]'); - } - - var checkout = StripeCheckout.configure({ - key: donationForm.getAttribute('data-checkout-publishable-key'), - image: donationForm.getAttribute('data-checkout-image'), - token: function(token) { - field('stripe_token').value = token.id; - field('stripe_token_type').value = token.type; - field('donor_email').value = token.email; - donationForm.submit(); - }, - bitcoin: true - }); - - donationForm.addEventListener('submit', function(e) { - if (!field('stripe_token').value) { - e.preventDefault(); - - var amountChoice = - donationForm.querySelector('input[name=amount]:checked'); - if (amountChoice.value === "custom") { - amountChoice = document.getElementById('amount-custom-value'); - } - - // Start parsing at the first digit in the string, to trim leading dollar - // signs and what have you. - var amountNumberString = (amountChoice.value.match(/[0-9].*/) || [""])[0]; - var amount = Math.floor(parseFloat(amountNumberString) * 100); - - if (!isNaN(amount)) { - field('amount').value = amountNumberString; - checkout.open({ - name: 'Dress to Impress', - description: 'Donation (thank you!)', - amount: amount, - panelLabel: "Donate" - }); - } - } - }); - - var toggle = document.getElementById('success-thanks-toggle-description'); - if (toggle) { - toggle.addEventListener('click', function() { - var desc = document.getElementById('description'); - var attr = 'data-show'; - if (desc.hasAttribute(attr)) { - desc.removeAttribute(attr); - } else { - desc.setAttribute(attr, true); - } - }); - } - - document.getElementById('amount-custom').addEventListener('change', function(e) { - if (e.target.checked) { - document.getElementById('amount-custom-value').focus(); - } - }); -})(); diff --git a/app/assets/javascripts/items/item_header.js b/app/assets/javascripts/items/item_header.js index 05035428..a009262c 100644 --- a/app/assets/javascripts/items/item_header.js +++ b/app/assets/javascripts/items/item_header.js @@ -3,22 +3,17 @@ function setFormStateCookie(value) { document.cookie = `DTIItemPageUserListsFormState=${value};max-age=${thirtyDays}`; } -const headers = document.querySelectorAll(".item-header"); -for (const header of headers) { - try { +document.addEventListener("click", (event) => { + if (event.target.matches(".item-header .user-lists-form-opener")) { + const header = event.target.closest(".item-header"); const form = header.querySelector(".user-lists-form"); - const opener = header.querySelector(".user-lists-form-opener"); - opener.addEventListener("click", (event) => { - if (form.hasAttribute("hidden")) { - form.removeAttribute("hidden"); - setFormStateCookie("open"); - } else { - form.setAttribute("hidden", ""); - setFormStateCookie("closed"); - } - event.preventDefault(); - }); - } catch (error) { - console.error(`Error applying dialog behavior to item header:`, error); + if (form.hasAttribute("hidden")) { + form.removeAttribute("hidden"); + setFormStateCookie("open"); + } else { + form.setAttribute("hidden", ""); + setFormStateCookie("closed"); + } + event.preventDefault(); } -} +}); diff --git a/app/assets/javascripts/outfits/index.js b/app/assets/javascripts/outfits/index.js deleted file mode 100644 index 8ced8dac..00000000 --- a/app/assets/javascripts/outfits/index.js +++ /dev/null @@ -1,3 +0,0 @@ -$("form.button_to input[type=submit]").click(function (e) { - if (!confirm(this.getAttribute("data-confirm"))) e.preventDefault(); -}); diff --git a/app/assets/javascripts/outfits/new.js b/app/assets/javascripts/outfits/new.js index 89827a0e..50dc4cf3 100644 --- a/app/assets/javascripts/outfits/new.js +++ b/app/assets/javascripts/outfits/new.js @@ -194,7 +194,7 @@ Preview.updateWithName(name_el); name_el.keyup(function () { - if (previewWithNameTimeout) { + if (previewWithNameTimeout && Preview.Job.current) { clearTimeout(previewWithNameTimeout); Preview.Job.current.loading = false; } diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 84f7dfe1..21e10c96 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -114,7 +114,7 @@ module ApplicationHelper def include_javascript_libraries(*library_names) raw(library_names.inject('') do |html, name| - html + javascript_include_tag(JAVASCRIPT_LIBRARIES[name]) + html + javascript_include_tag(JAVASCRIPT_LIBRARIES[name], defer: true) end) end diff --git a/app/views/closet_hangers/index.html.haml b/app/views/closet_hangers/index.html.haml index 151cbc1b..ddcd87ed 100644 --- a/app/views/closet_hangers/index.html.haml +++ b/app/views/closet_hangers/index.html.haml @@ -153,7 +153,10 @@ - content_for :javascripts do = include_javascript_libraries :jquery, :jquery_tmpl = javascript_include_tag 'ajax_auth', 'lib/jquery.ui', 'lib/jquery.jgrowl', - 'closet_hangers/index' + defer: true + +- content_for :javascripts_body do + = javascript_include_tag 'closet_hangers/index', defer: true - if public_perspective? && user_signed_in? - content_for :meta do diff --git a/app/views/fundraising/campaigns/show.html.haml b/app/views/fundraising/campaigns/show.html.haml index 39d0560e..560034c1 100644 --- a/app/views/fundraising/campaigns/show.html.haml +++ b/app/views/fundraising/campaigns/show.html.haml @@ -123,9 +123,5 @@ #{mail_to 'webmaster@openneo.net', "Please email us for more information."} Thank you!! -- content_for :javascripts do - = javascript_include_tag 'https://checkout.stripe.com/checkout.js', - 'fundraising/campaigns/show' - - content_for :stylesheets do = stylesheet_link_tag 'fundraising/campaigns/show' diff --git a/app/views/fundraising/donations/show.html.haml b/app/views/fundraising/donations/show.html.haml index 897a22ed..66976193 100644 --- a/app/views/fundraising/donations/show.html.haml +++ b/app/views/fundraising/donations/show.html.haml @@ -43,7 +43,9 @@ - content_for :javascripts do = include_javascript_libraries :jquery - = javascript_include_tag 'fundraising/donations/show' + +- content_for :javascripts_body do + = javascript_include_tag 'fundraising/donations/show', defer: true - content_for :stylesheets do = stylesheet_link_tag 'fundraising/donations/show' diff --git a/app/views/items/_item_header.html.haml b/app/views/items/_item_header.html.haml index 7d1fe303..d1a68f4f 100644 --- a/app/views/items/_item_header.html.haml +++ b/app/views/items/_item_header.html.haml @@ -96,4 +96,4 @@ 'data-is-current' => current_subpage == 'trades_seeking' - content_for :javascripts do - = javascript_include_tag 'items/item_header' + = javascript_include_tag 'items/item_header', defer: true diff --git a/app/views/items/show.html.haml b/app/views/items/show.html.haml index 87468289..84f3aeea 100644 --- a/app/views/items/show.html.haml +++ b/app/views/items/show.html.haml @@ -23,6 +23,6 @@ %li= link_to(contributor.name, user_contributions_path(contributor)) + format_contribution_count(count) %footer= t '.contributors.footer' -- content_for :javascripts do +- content_for :javascripts_body do = javascript_include_tag 'item-page', defer: true diff --git a/app/views/layouts/application.html.haml b/app/views/layouts/application.html.haml index 13d27459..50fa6fd6 100644 --- a/app/views/layouts/application.html.haml +++ b/app/views/layouts/application.html.haml @@ -21,6 +21,8 @@ = signed_in_meta_tag - if user_signed_in? = current_user_id_meta_tag + = javascript_include_tag 'application', defer: true + = yield :javascripts = yield :head %body{:class => body_class} #container @@ -73,7 +75,9 @@ %li= mail_to contact_email, t('.footer.email') %p= t '.footer.copyright', :year => Date.today.year - - = javascript_include_tag 'application' - = yield(:javascripts) + -# For scripts that expect to get to just execute once every time the page + -# loads. For future JS, it would be advisable to write it such that it's + -# okay for Turbo to run it once at the start of the page, then listen to + -# `turbo:load` events or use inline scripts to actually _do_ things. + = yield :javascripts_body diff --git a/app/views/outfits/index.html.haml b/app/views/outfits/index.html.haml index 1858c578..984be479 100644 --- a/app/views/outfits/index.html.haml +++ b/app/views/outfits/index.html.haml @@ -6,8 +6,3 @@ %ul#outfits= render @outfits - else %p= twl '.no_outfits', :start_link_url => root_path - -- content_for :javascripts do - = include_javascript_libraries :jquery - = javascript_include_tag 'outfits/index' - diff --git a/app/views/outfits/new.html.haml b/app/views/outfits/new.html.haml index 791e069d..5f915c00 100644 --- a/app/views/outfits/new.html.haml +++ b/app/views/outfits/new.html.haml @@ -124,4 +124,7 @@ - content_for :javascripts do = include_javascript_libraries :jquery20, :jquery_tmpl - = javascript_include_tag 'ajax_auth', 'lib/jquery.timeago', 'outfits/new' \ No newline at end of file + = javascript_include_tag 'ajax_auth', 'lib/jquery.timeago', defer: true + +- content_for :javascripts_body do + = javascript_include_tag 'outfits/new', defer: true \ No newline at end of file diff --git a/app/views/pets/bulk.html.haml b/app/views/pets/bulk.html.haml index 1e4ea308..44d7a0aa 100644 --- a/app/views/pets/bulk.html.haml +++ b/app/views/pets/bulk.html.haml @@ -79,4 +79,7 @@ - content_for :javascripts do = include_javascript_libraries :jquery, :jquery_tmpl - = javascript_include_tag 'ajax_auth', 'pets/bulk' + = javascript_include_tag 'ajax_auth', defer: true + +- content_for :javascripts_body do + = javascript_include_tag 'pets/bulk', defer: true