adsbim / adsb-im-web

content of the adsb.im website
Other
2 stars 3 forks source link

Remove jQuery #4

Closed afoeder closed 4 months ago

afoeder commented 4 months ago

~This removes fetching the jQuery library as well as the only place when it was used, which did not work anyways: expanding the FAQ entry when the page was called with a hash.~

This removes the use of jQuery by replacing the only place where it was used with vanilla JS, and it omits fetching the library altogether.

afoeder commented 4 months ago

on the Howto page, there's a link to /faq#collapsewifisupport. This needs to be backwards compatible.

dirkhh commented 4 months ago

This worked just a few weeks ago.

I'm having a surgical procedure today (which is why I was up at 3am, merging your PR). So I'm not sure when I'll be able to look at this

afoeder commented 4 months ago

oh no hurries and good luck!

dirkhh commented 4 months ago

Still waiting for my procedure... I used the time to merge and deploy your other PRs. This one I want to sit on for a moment in order to figure out what's going on there.

Somewhere else you wondered how closely this is linked to the 'live' web site... I quite literally created a git subproject and this project is now checked out as part of my overall website setup (which includes all of the other backend stuff, including routes, update notifications, etc). So the moment I pull from this repo, the updates get deployed 😀

afoeder commented 4 months ago

briefly from mobile: it did work before, I just expected it to also jump to the right spot which it doesn‘t. Perhaps some preventDefault() happening. I‘ve still adjusted it to vanilla JS to get rid of the jQuery dependency. still all the best for your procedure!

Sent from Outlook for iOShttps://aka.ms/o0ukef


From: Dirk Hohndel @.> Sent: Tuesday, April 23, 2024 6:16:51 PM To: dirkhh/adsb-im-web @.> Cc: Adrian Föder @.>; Author @.> Subject: Re: [dirkhh/adsb-im-web] Remove jQuery (PR #4)

Still waiting for my procedure... I used the time to merge and deploy your other PRs. This one I want to sit on for a moment in order to figure out what's going on there.

Somewhere else you wondered how closely this is linked to the 'live' web site... I quite literally created a git subproject and this project is now checked out as part of my overall website setup (which includes all of the other backend stuff, including routes, update notifications, etc). So the moment I pull from this repo, the updates get deployed 😀

— Reply to this email directly, view it on GitHubhttps://github.com/dirkhh/adsb-im-web/pull/4#issuecomment-2072849336, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAISWMEMVHJJFTQLBQARCSLY62CPHAVCNFSM6AAAAABGU2KMF2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANZSHA2DSMZTGY. You are receiving this because you authored the thread.Message ID: @.***>

dirkhh commented 4 months ago

So... this is where you clearly are way ahead of me when it comes to Javascript... I'm mostly a "copy and paste and then mess with it until it appears to work" JS programmer :) In other words... can you explain to me what was broken and how? I understand that you replaced jQuery with plain JS (which I always find so much harder to read, but I get the advantage of course). But I'm not sure what if anything that changes from a functionality perspective.

afoeder commented 4 months ago

can you explain to me what was broken and how

there wasn't really anything broken, that was my mistake. I will adjust the commit message when I rebase the PR. What led to my assumption that it didn't work was that the page didn't jump to the open entry. Check https://adsb.im/faq#collapserawiron for example: the item expands, but the viewport isn't scrolled to the target.

So bottom line: I just replaced jQ with vanilla JS since these two lines won't justify pulling the whole jQuery lib.