@SutonToch
Posted
I didn't think I'd come back so soon, but it didn't let me rest, so here we are.
I only did very light reading on jQuery, so take my notes with a grain of salt. But my notes will most definitely make your code more readable.
So let's get cracking:
- Your first and second if-clause repeat each other, in that both want to close the currently open dropdown. In those cases, it's always best to create a function that handles that part for you, e.g.:
function closeDropdown(activeDropdown) {
$(activeDropdown).removeClass("active-dropdown");
$(activeDropdown)
.find(".container_right_dropdown_arrow")
.removeClass("rotate-arrow");
allTexts.each(function (i) {
$(allTexts[i]).hide(300);
});
}
- You could do the same thing for opening a dropdown, just to stay consistent. E.g.:
function openDropdown(clickedDropdown) {
$(clickedDropdown).addClass("active-dropdown");
$(clickedDropdown)
.find(".container_right_dropdown_arrow")
.addClass("rotate-arrow");
allTexts.each(function (i) {
if ($(allTexts[i].parentElement).hasClass("active-dropdown")) {
$(allTexts[i]).show(300);
}
});
}
- Let's tackle the
allTexts.each(...)
next. With that you loop through every single text, to find the correct one, and then show/hide it. We don't need to loop everything, because we already have the parent. To show the text when opening, you could write the following instead:
$(clickedDropdown).find(".container_right_dropdown-text").show(300);
- The great thing is, you already used the .find() method for the arrow, so we're staying consistent. Another bonus, we don't need the global constant allTexts anymore. You can do something very similar with closing the texts too.
- Let's address your second if-clause next, where I can only assume that with
$(dropdown).hasClass("active-dropdown")
jQuery goes through every single.container_right_dropdown_each-faq
and looks if it is the active dropdown. It would be more efficient if we would store the active dropdown in a global variable. This way we could close it directly usingcloseDropdown(activeDropdown)
. - Obviously, a few tweaks are needed for that to work. When opening a dropdown, that clicked dropdown must be saved to the active dropdown. When closing a dropdown, the active dropdown must be removed (e.g. reassign with null).
- In your first if-clause, we can now also check if the clickedDropdown is our activeDropdown via
$(clickedDropdown).is(activeDropdown)
. - Now our internal logic when a dropdown element is clicked looks more like this:
dropdown.click(function (e) {
// saves the element clicked in a variable
let clickedDropdown = e.currentTarget;
if ($(clickedDropdown).is(activeDropdown)) {
// Clicked on the only open dropdown
closeDropdown(clickedDropdown);
} else {
// Clicked on a different dropdown, than the open one -> close the original open one
closeDropdown(activeDropdown);
// Open the clicked dropdown
openDropdown(clickedDropdown)
}
});
- Puh.. this was probably a lot to take in. I'd recommend retracing my written steps in your own pace again.
A few takeaways:
- Think about what functionalities you want to implement (e.g. opening and closing a dropdown) and write a function for those.
- Then create your logic, that calls the functions with the correct parameters when they are needed.
- Make use of the information you already have (e.g. the clicked element and therefore the text of its child)
- Usually that's an iterative process, because it's very rare that the original plan pans out exactly like one hoped, or you find an even more efficient way.
I hope I was able to help you with this :), but don't forget that it's very normal for the first iteration of code to "just work". I'm pretty sure my proposed improvements can be improved even further. Feel free to ask me any further questions, I'm always happy to help.
Keep on going!
Marked as helpful
@OdaSolaDev
Posted
Hey there, @SutonToch
Both of your comments were really helpful. Especially the one for jQuery. My code for this challenge (and I hope for the ones to come too) is much clearer now.
You made REALLY good notes about my code.
Thank you! You actually helped me A LOT. ^^