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

Community feedback

• 

  Kerri-Ann•330

  @Kerri-AnnBatesPosted about 4 years ago

  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 :)

  2

  ---

  

  Josh Prentice•235

  @jfprenticePosted about 4 years ago

  Thanks for looking at my code! I definitely overlooked that I was using the assignment operator - I'll go back and see if that was the culprit! Thanks again!

  1

  

  Kerri-Ann•330

  @Kerri-AnnBatesPosted about 4 years ago

  @jfprentice You're welcome! I actually read up on this myself after seeing your code and found out that, the solution I posted in my previous comment would not work either because x.style will only work to check inline styles and not styles from an external stylesheet.

  After some research I came across this documentation: (https://developer.mozilla.org/en-US/docs/Web/API/Window/getComputedStyle) which looks like is a more likely way to check if an element's css property has a specific value. Something else you can look at. I learned something new with this too!

  1

  

  Josh Prentice•235

  @jfprenticePosted about 4 years ago

  @Kerri-AnnBates I did go back and change the operator but with no result. That's good to know about .style though! I have a lot to learn when it comes to JS and DOM manipulation. Thanks for the link!

  0
• 

  Roman Filenko•3,335

  @rfilenkoPosted about 4 years ago

  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

  1

  ---

  

  Josh Prentice•235

  @jfprenticePosted about 4 years ago

  @rfilenko you are 100% correct - I'll need to go back and make a few changes to my share div positioning but that definitely centered it! Thank you!

  0
• 

  T

  ApplePieGiraffe•30,545

  @ApplePieGiraffePosted about 4 years ago

  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)! 😁

  1

  ---

  

  Josh Prentice•235

  @jfprenticePosted about 4 years ago

  @ApplePieGiraffe that's what I was trying to achieve with the padding haha I thought I had it - I always have trouble getting things vertically centered. Using auto margins will get it horizontally centered but the top of the card is still at the top of the page. I tried applying flexbox to the body with justify-content and align-items as center but I ended up with the same result as the auto margins. I'm going to do a little more research and see how I can fix it. Thanks for checking out my work!

  0