Analizowałem właśnie moją odpowiedź do Twojego wcześniejszego posta i widzę, że jak proponowałem pozbyłeś się jQuery na rzecz vanillaJS :) Duży plusik za to, jak pisałem wtedy, wg mnie pokazujesz w ten sposób, że znasz JS, a jeśli tak to jQuery wiem, że nawet jeśli nie znasz to szybko się nauczysz, bo wiesz co i jak.
Mogę dać ewentualnie jeszcze parę drobiazgów do JS:
- nie musisz robić tego window.onload, daj po prostu kod JS przed zamknięciem body albo z atrybutem defer.
- Kod:
function AddModalClass(Text)
{
let AddClass = Text.replace("<img ",'<img class="ModalImg" ')
return AddClass;
}
Dwie kwestie. Uważaj z metodą String.prototype.replace stosując ją z ciągiem tekstowym jako elementem szukanym, ponieważ analizuje ona tylko pierwsze wystąpienie tego elementu w całym ciągu. Tutaj jest oki, ale tak na przyszłość, jeśli chciałbyś wyszukać kilka elementów i je pozamieniać to zainteresuj się wyrażeniami regularnymi.
o taka uwaga poboczna, a główna to dlaczego nie classList.add wywołana na elemencie img (odpowiednio pobranej referencji do niego)?
- Kod:
function AddModals()
{
let ModalLenght = document.getElementsByClassName("Modal").length;
for(let i=0;i<ModalLenght;i++)
{
document.getElementsByClassName("Modal")[i].addEventListener("click",function(){
document.getElementById("Modal").innerHTML = '<div id="ModalBG" onclick="AddActive();"></div>'+AddModalClass(document.getElementsByClassName("Modal")[i].innerHTML);
document.getElementById("Modal").classList.add("active");
});
}
}
Zwróć uwagę, że kilka razy pobierasz od nowa referencję do obiektu o klasie "Modal". To zła praktyka. Pobierz ją raz i zapamiętaj np. w zmiennej
const modal = document.querySelectorAll('.modal')
i teraz używaj tej zmiennej, np. "modal.length", "modal[i]" itp. Ma to też po za wydajnością drugi plus - jeśli z jakiś powodów zmienisz klasę elementu to zmieniasz tylko w jednym miejscu, a tak w ogóle to można by zrobić funkcję bardziej uniwersalną i klasę ".modal" przekazywać jako argument funkcji :)
-
[i].addEventListener("click",function(){
document.getElementById("Modal").innerHTML = '<div id="ModalBG" onclick="AddActive();">
dlaczego mieszasz addEventListener z onclick i do tego? Dodatkowo jeśli tworzysz kilka elementów DOM na raz to poczytaj o createDocumentFragment i po prostu dodaj potem w odpowiednim elemencie ten nowo stworzony. Bardzo ostrożnie używaj innerHTML, a najlepiej w ogóle tego nie używaj. Jeśli chcesz zmienić tylko "tekst" to użyj textContent, a jeśli coś więcej to polecam odpowiednie metody do tego np. metody classList.xxx czy createElement itp.
-
Tak na koniec jeszcze mała sugestia, aby nie wywoływać funkcji, których wcześniej nie zadeklarowałeś. Jeśli używasz deklaracji w formie:
function fn() {}
to będzie to działać, ponieważ zajdzie tu hoisting funkcji, ale jeśli dałbyś:
const fn = function () {};
//albo:
const fn = () => {};
to będzie błąd, ponieważ const nie podlega takiemu samemu zjawiskowi hositingu. Twój kod działa, ale to taka sugestia, ja osobiście stosuję się do zasady, że każda funkcja i zmienna jakiej używam musi być wcześniej zadeklarowana/zaimportowana itp. To znacznie ułatwia potem czytanie i analizę kodu, bo nie musisz skakać po całym kodzie.
To tak na szynko w przerwie kawowej :) Generalnie spoko, na prawdę plusik za przejście na czysty JS - sam widzisz, że wcale kod nie jest jakoś strasznie dłuższy i pewnie wielu ciekawych rzeczy się mogłeś nauczyć :)
jQuery nie jest złe, co więcej jest używane w wielu cms i istniejących stronkach także będąc w webie na pewno jeszcze nie raz się spotkasz z tą biblioteką, ale warto znać dobrze czysty JS, żeby zawsze móc samemu zrobić coś, czego jQuery może nie mieć :)
Pozdrawiam i życzę powodzenia w rozwoju!
