I'd like to suggest the opposite of Sᴀᴍ Onᴇᴌᴀ: Since you are using jQuery for selecting only, you can drop it altogether.
const icons = $(".magic");
becomes either
const icons = document.getElementsByClassgetElementsByClassName("magic");
or
const icons = document.querySelectorAll(".magic");
However you shouldn't use on...
properties to assign event handlers. on...
properties can only hold a single handler so if you or another script would attempt to assign another click handler then they'd overwrite each other. Instead use addEventListener
.
Alternatively to avoid assigning seperate event handlers to each icon you could use event delegation. This means assign an single event handler to a surrounding element (or simply document
) and check the target element:
document.addEventListener("click", function (event) {
if (event.target.classList.contains("magic")) {
event.target.classlist.toggle("enabled");
}
};
Finally you could avoid using JavaScript altogether by using an HTML element that has the toggle functionality built in: a checkbox:
<label class="magic-wrapper">
<input type="checkbox"><span class="magic"><i class="fas fa-star fa-5x"></i></span>
</label>
Hide the actual checkbox with:
.magic-wrapper > input {
opacity: 0;
position: absolute;
}
(This is more accessable than just using display: none;)
And replace the selector .enabled
with input:checked + .magic
in the CSS.
Complete example: https://jsfiddle.net/rhy6gfn4/
A small points about the CSS:
You should select not just .enabled
but use .magic.enabled
, because then it's more obvious that these rules belong to the animated icons. Also "enabled" is a common class name and you don't want those styles apply to unrelated elements.
It would be a tiniest bit more performant to select the i
elements using .magic > i
and not just .magic i
.
I'd like to suggest the opposite of Sᴀᴍ Onᴇᴌᴀ: Since you are using jQuery for selecting only, you can drop it altogether.
const icons = $(".magic");
becomes either
const icons = document.getElementsByClass("magic");
or
const icons = document.querySelectorAll(".magic");
However you shouldn't use on...
properties to assign event handlers. on...
properties can only hold a single handler so if you or another script would attempt to assign another click handler then they'd overwrite each other. Instead use addEventListener
.
Alternatively to avoid assigning seperate event handlers to each icon you could use event delegation. This means assign an single event handler to a surrounding element (or simply document
) and check the target element:
document.addEventListener("click", function (event) {
if (event.target.classList.contains("magic")) {
event.target.classlist.toggle("enabled");
}
};
Finally you could avoid using JavaScript altogether by using an HTML element that has the toggle functionality built in: a checkbox:
<label class="magic-wrapper">
<input type="checkbox"><span class="magic"><i class="fas fa-star fa-5x"></i></span>
</label>
Hide the actual checkbox with:
.magic-wrapper > input {
opacity: 0;
position: absolute;
}
(This is more accessable than just using display: none;)
And replace the selector .enabled
with input:checked + .magic
in the CSS.
Complete example: https://jsfiddle.net/rhy6gfn4/
A small points about the CSS:
You should select not just .enabled
but use .magic.enabled
, because then it's more obvious that these rules belong to the animated icons. Also "enabled" is a common class name and you don't want those styles apply to unrelated elements.
It would be a tiniest bit more performant to select the i
elements using .magic > i
and not just .magic i
.
I'd like to suggest the opposite of Sᴀᴍ Onᴇᴌᴀ: Since you are using jQuery for selecting only, you can drop it altogether.
const icons = $(".magic");
becomes either
const icons = document.getElementsByClassName("magic");
or
const icons = document.querySelectorAll(".magic");
However you shouldn't use on...
properties to assign event handlers. on...
properties can only hold a single handler so if you or another script would attempt to assign another click handler then they'd overwrite each other. Instead use addEventListener
.
Alternatively to avoid assigning seperate event handlers to each icon you could use event delegation. This means assign an single event handler to a surrounding element (or simply document
) and check the target element:
document.addEventListener("click", function (event) {
if (event.target.classList.contains("magic")) {
event.target.classlist.toggle("enabled");
}
};
Finally you could avoid using JavaScript altogether by using an HTML element that has the toggle functionality built in: a checkbox:
<label class="magic-wrapper">
<input type="checkbox"><span class="magic"><i class="fas fa-star fa-5x"></i></span>
</label>
Hide the actual checkbox with:
.magic-wrapper > input {
opacity: 0;
position: absolute;
}
(This is more accessable than just using display: none;)
And replace the selector .enabled
with input:checked + .magic
in the CSS.
Complete example: https://jsfiddle.net/rhy6gfn4/
A small points about the CSS:
You should select not just .enabled
but use .magic.enabled
, because then it's more obvious that these rules belong to the animated icons. Also "enabled" is a common class name and you don't want those styles apply to unrelated elements.
It would be a tiniest bit more performant to select the i
elements using .magic > i
and not just .magic i
.
I'd like to suggestivsuggest the opposite of Sᴀᴍ Onᴇᴌᴀ: Since you are using jQuery for selecting only, you can drop it altogether.
const icons = $(".magic");
becomes either
const icons = document.getElementsByClass("magic");
or
const icons = document.querySelectorAll(".magic");
However you shouldn't use on...
properties to assign event handlers. on...
properties can only hold a single handler so if you or another script would attempt to assign another click handler then they'd overwrite each other. Instead use addEventListener
.
Alternatively to avoid assigning seperate event handlers to each icon you could use event delegation. This means assign an single event handler to a surrounding element (or simply document
) and check the target element:
document.addEventListener("click", function (event) {
if (event.target.classList.contains("magic")) {
event.target.classlist.toggle("enabled");
}
};
Finally you could avoid using JavaScript altogether by using an HTML element that has the toggle functionality built in: a checkbox:
<label class="magic-wrapper">
<input type="checkbox"><span class="magic"><i class="fas fa-star fa-5x"></i></span>
</label>
Hide the actual checkbox with:
.magic-wrapper > input {
opacity: 0;
position: absolute;
}
(This is more accessable than just using display: none;)
And replace the selector .enabled
with input:checked + .magic
in the CSS.
Complete example: https://jsfiddle.net/rhy6gfn4/
A small points about the CSS:
You should select not just .enabled
but use .magic.enabled
, because then it's more obvious that these rules belong to the animated icons. Also "enabled" is a common class name and you don't want those styles apply to unrelated elements.
It would be a tiniest bit more performant to select the i
elements using .magic > i
and not just .magic i
.
I'd like to suggestiv the opposite of Sᴀᴍ Onᴇᴌᴀ: Since you are using jQuery for selecting only, you can drop it altogether.
const icons = $(".magic");
becomes either
const icons = document.getElementsByClass("magic");
or
const icons = document.querySelectorAll(".magic");
However you shouldn't use on...
properties to assign event handlers. on...
properties can only hold a single handler so if you or another script would attempt to assign another click handler then they'd overwrite each other. Instead use addEventListener
.
Alternatively to avoid assigning seperate event handlers to each icon you could use event delegation. This means assign an single event handler to a surrounding element (or simply document
) and check the target element:
document.addEventListener("click", function (event) {
if (event.target.classList.contains("magic")) {
event.target.classlist.toggle("enabled");
}
};
Finally you could avoid using JavaScript altogether by using an HTML element that has the toggle functionality built in: a checkbox:
<label class="magic-wrapper">
<input type="checkbox"><span class="magic"><i class="fas fa-star fa-5x"></i></span>
</label>
Hide the actual checkbox with:
.magic-wrapper > input {
opacity: 0;
position: absolute;
}
(This is more accessable than just using display: none;)
And replace the selector .enabled
with input:checked + .magic
in the CSS.
Complete example: https://jsfiddle.net/rhy6gfn4/
A small points about the CSS:
You should select not just .enabled
but use .magic.enabled
, because then it's more obvious that these rules belong to the animated icons. Also "enabled" is a common class name and you don't want those styles apply to unrelated elements.
It would be a tiniest bit more performant to select the i
elements using .magic > i
and not just .magic i
.
I'd like to suggest the opposite of Sᴀᴍ Onᴇᴌᴀ: Since you are using jQuery for selecting only, you can drop it altogether.
const icons = $(".magic");
becomes either
const icons = document.getElementsByClass("magic");
or
const icons = document.querySelectorAll(".magic");
However you shouldn't use on...
properties to assign event handlers. on...
properties can only hold a single handler so if you or another script would attempt to assign another click handler then they'd overwrite each other. Instead use addEventListener
.
Alternatively to avoid assigning seperate event handlers to each icon you could use event delegation. This means assign an single event handler to a surrounding element (or simply document
) and check the target element:
document.addEventListener("click", function (event) {
if (event.target.classList.contains("magic")) {
event.target.classlist.toggle("enabled");
}
};
Finally you could avoid using JavaScript altogether by using an HTML element that has the toggle functionality built in: a checkbox:
<label class="magic-wrapper">
<input type="checkbox"><span class="magic"><i class="fas fa-star fa-5x"></i></span>
</label>
Hide the actual checkbox with:
.magic-wrapper > input {
opacity: 0;
position: absolute;
}
(This is more accessable than just using display: none;)
And replace the selector .enabled
with input:checked + .magic
in the CSS.
Complete example: https://jsfiddle.net/rhy6gfn4/
A small points about the CSS:
You should select not just .enabled
but use .magic.enabled
, because then it's more obvious that these rules belong to the animated icons. Also "enabled" is a common class name and you don't want those styles apply to unrelated elements.
It would be a tiniest bit more performant to select the i
elements using .magic > i
and not just .magic i
.
I'd like to suggestiv the opposite of Sᴀᴍ Onᴇᴌᴀ: Since you are using jQuery for selecting only, you can drop it altogether.
const icons = $(".magic");
becomes either
const icons = document.getElementsByClass("magic");
or
const icons = document.querySelectorAll(".magic");
However you shouldn't use on...
properties to assign event handlers. on...
properties can only hold a single handler so if you or another script would attempt to assign another click handler then they'd overwrite each other. Instead use addEventListener
.
Alternatively to avoid assigning seperate event handlers to each icon you could use event delegation. This means assign an single event handler to a surrounding element (or simply document
) and check the target element:
document.addEventListener("click", function (event) {
if (event.target.classList.contains("magic")) {
event.target.classlist.toggle("enabled");
}
};
Finally you could avoid using JavaScript altogether by using an HTML element that has the toggle functionality built in: a checkbox:
<label class="magic-wrapper">
<input type="checkbox"><span class="magic"><i class="fas fa-star fa-5x"></i></span>
</label>
Hide the actual checkbox with:
.magic-wrapper > input {
opacity: 0;
position: absolute;
}
(This is more accessable than just using display: none;)
And replace the selector .enabled
with input:checked + .magic
in the CSS.
Complete example: https://jsfiddle.net/rhy6gfn4/
A small points about the CSS:
You should select not just .enabled
but use .magic.enabled
, because then it's more obvious that these rules belong to the animated icons. Also "enabled" is a common class name and you don't want those styles apply to unrelated elements.
It would be a tiniest bit more performant to select the i
elements using .magic > i
and not just .magic i
.