Article Preview Component using HTML/CSS - Flexbox and a little JS

Solution retrospective
I am always open to feedback and appreciate any and all critiques!
This was the first project I've done that has used JS - I've been wanting start incorporating JS into my projects but I wasn't sure if I was good enough with HTML/CSS yet to branch off to JS. I also don't have much experience with JS DOM manipulation.
Even though the JS part was really simple, I did have a few issues getting it to work properly - tried using document.getElementsByClassName but couldn't get it to work properly so I had to add a couple of id tags and use .getElementById. I also had an issue toggling the visibility of my share div. I tried using a ternary conditional statement and then just an if/else, both would only make the div visible but not back to hidden. (I left my ternary conditional in the index.js file as a comment if someone wants to look and see where I went wrong) I ended up making a separate class for visibility and used .toggle() to switch between the classes.
As before, any tips/tricks/critiques are welcome - your feedback really helps me improve!
Thanks for looking!
Please log in to post a comment
Log in with GitHubCommunity feedback
- @Kerri-AnnBates
Hi, I think this looks great! Good job. I saw a comment in your js file about this line of code not working:
x.style.visibility ="hidden" ? "visible" : "hidden";
It probably did not work because your condition is assigning the visibility style to "hidden". I think what you're looking for is the equality operator. Using
==
instead of=
Like so:
x.style.visibility =="hidden" ? "visible" : "hidden";
That may work. Hope that makes sense and that it helps :)
- @rfilenko
Hey, good job on this one. To make a card be centered on a page you missed one important property - min-height. So set on a body {display: flex; justify-content: center; align-items: center; min-height: 100vh;}. Also wrap all other child elements inside one container, that should do the trick😉, also disable margins, paddings as well.
Cheers, Roman
- @ApplePieGiraffe
Hey, Josh Prentice! 👋
Nice job on this challenge! Your solution looks good, responds well, and your social media share button works great! 🙌
I only suggest,
- Centering the card in the viewport at all screen sizes, either by using auto margins or flexbox or something. Currently, the large padding at the top of the page pushes the card off center at some screen sizes.
Keep coding (and happy coding, too)! 😁
Join 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