Design comparison
Solution retrospective
Is there a way to have the Javascript toggle between displaying the divs? For example, on the desktop screen size I added an "x" icon and new function for closing the pop-up because the share icon already had a function for changing the display from "none" to "flex".
Community feedback
- @vanzasetiaPosted over 2 years ago
Hi, Adrianna! 👋
I strongly recommend using
button
element insteaddiv
. 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 helpful1@adrianna-thomasPosted over 2 years ago@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 adiv
. I also tried using variables but it wasn't working so I changed itdocument.querySelector
, but I'll try your suggestions and see if I can get it working.0@vanzasetiaPosted over 2 years ago@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"; });
0 - I highly suggest using variables in JavaScript to prevent repetition like keep selecting the same element with
Please log in to post a comment
Log in with GitHubJoin our Discord community
Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!
Join our Discord