@vanzasetia
Posted
Hi, Adrianna! 👋
I strongly recommend using button element instead div. The element has interactivity so it's best to use an interactive element.
Regarding the question that you have, I made the toggle functionality where if the pop-up is showing up then to close the pop-up then the user has to click the share button again to close it and vice versa. You can see my solution as your reference.
For the feedback.
- I highly suggest using variables in JavaScript to prevent repetition like keep selecting the same element with
document.querySelector. Not only that, but it will make also make it easier to maintain because you need to only change the code on the variable instead of changing it one by one. - It's a good practice to separate the function from the event listener. This way, you can give a descriptive name to your function which makes it easier for other developers (including yourself in the future) to understand what these lines of code are doing.
That's it! Hope this helps
Marked as helpful
@adrianna-thomas
Posted
@vanzasetia Thank you for the feedback! When I submitted the challenge with a button element the html validation gave me an error saying that buttons should have text/can't be empty. So I changed it to a div. I also tried using variables but it wasn't working so I changed it document.querySelector, but I'll try your suggestions and see if I can get it working.
@vanzasetia
Posted
@adrianna-thomas You can give it a sr-only text. Basically, it's visually hidden however it's visible for screen reader users. This way the share button is accessible to everyone.
That's strange. When you are using variables, how did you write the code? If you did something like this, there's should not be any problem.
const shareBtn = document.querySelector(".share-btn");
const openShare = document.querySelector(".open-share");
shareBtn.addEventListener("click", function() {
openShare.style.display = "none";
openShare.style.display = "flex";
});
