Skip to content
OverviewSolutions
11
Comments
43
Heli0s
@zeerobit

All comments

  • Heli0s•670
    @zeerobitPosted about 2 years ago

    Well done completing your first grid challenge. I got a few pointers:

    • your h1 and h2 are actually paragraphs and should be inside of blockquote.
    • Replace your section tags with figure tags and use figcaption to wrap the user info, click this link to read more about these elements.
    • none of the images has alt and that's bad for accessibility.
    • The cards need a box-shadow

    Happy coding !!!

    0
  • Ritik Raj•20
    @ritik48Submitted over 2 years ago

    This project made me learn Grid Layout more thoroughly. Though it was little difficult for me to understand the grid's different properties still I am really glad that I took this project and completed it.

    Testimonials Grid Section

    1
    Heli0s•670
    @zeerobitPosted over 2 years ago

    well done on completing this challenge, however i have a few remarks:

    • use more semantics in your html than just div.. look into figure, figcaption and blockquote which are more suitable semantics for this challenge. Basically each card should be wrapped with the figure tag instead of div, the photo along with the name and title should be wrapped in figcaption and the paragraphs in blockquote.
    • You should research about grid-areas as it would be a lot easier for the desktop layout, you would use a lot less code. Here is an example of the code for my grid desktop layout:
        display: grid;
    grid-template-columns: repeat(4, 1fr);
    column-gap: 1.2rem;
    grid-template-areas:
      "card1 card1  card2 card5"
      "card3 card4  card4 card5 ";
  }

    hope this helps, happy coding

    1
  • George Stawowczyk•210
    @Jorgus1710Submitted over 2 years ago

    A fun project that really made me explore in depth what exactly pseudo classes are (::before & ::after), as well it gave insight on how to use position: relative and absolute. I'm not going to lie, achieving the hover state on the main image was far more difficult than I had initially anticipated. It took me 2 days of trial & error, as well as learning to finally achieve the desired result.

    Question(s):

    1. When hovering over the main image, you can see the translucent cyan overlay goes a bit beyond the lower border of the actual image or the image wrapper. Its not by much, and certainly I can get away with this result as is - but I'd love to have insight as to why this is happening.

    2. For the hover states on the main image I used position absolute & relative to center the "eye" SVG icon to its parent wrapper. As well for the cyan overlay which is translucent, I used an empty div in the parent container of the main image, to which I then applied a ::before pseudoclass. If anyone has an alternative or ideally more simplified method of achieving these hover affects please let me know.

    NFT card with hover states.

    2
    Heli0s•670
    @zeerobitPosted over 2 years ago

    Great job completing this challenge.

    1- in regards to your first question you can fix that by adding display: block; on the .main-img class. You should use max-width and not width on images, so it should be max-width: 100%;

    2- Another way you can center the icon-view is to remove the pseudo element completely, add the icon-view image inside of the .overlay div then use position absolute to move it on top of the .img-wrapper. use flexbox to center the icon-view . Remove the properties you have under .hover-img and simply add display: block; max-width: 100%;

    3- The images are supposed to be interactive thus you should wrap them in anchor tags.

    4- no need to add alt description for the 2 images since they're decorative images, add aria-hidden:"true" to hide them from assistive technology

    5- Use rem instead of px since it's not scalable

    6- The creator section could be wrapped in figure and figcaption for better semantics

    7- the .attribution should be wrapped in footer

    Hope these help, if you have questions feel free to ask. Happy coding !!!

    Marked as helpful
    0
  • Martha Eason•30
    @marthaeasonSubmitted over 2 years ago

    Any advice is appreciated, especially with my flex-box as it is one of my first experiments learning this!

    I can't figure out how to get the NFT Card image to change its state for :hover. - any tips on accomplishing this? I tried overlaying the aqua background color with opacity and z-index but that didn't work. Thank you!

    NFT Card challenge using flex box

    2
    Heli0s•670
    @zeerobitPosted over 2 years ago

    Hi Martha, here are a few pointers:

    • You don't necessarily need to wrap the equilibrium image in 2 divs instead use one of the divs to also wrap the icon-view image. Here is an example:
    <div class="img__container">
        <a href="#">
          <img src="images/image-equilibrium.jpg" class="eq__img" alt="" aria-hidden="true" />
        </a>
        <div class="overlay">
          <a href="#"><img src="images/icon-view.svg" alt="" aria-hidden="true" / class="icon-view"></a>
        </div>
</div>
    • In order to get the hover effect, add position relative to the image-container and on the overlay use position absolute so you can place the overlay on top of the image-container, convert the hsl background color to rgb [rgb(0, 255, 247, 0.4);] which lets you add the opacity for the transparency color effect, use flexbox to center the icon-view image then add opacity: 0; to hide the overlay. Then add opacity: 1; on hover so the overlay appears when you hover
    .overlay:hover {
  opacity: 1;
  cursor: pointer;
}
    • ETH is missing the 0.041 in the front
    • The creator part can be wrapped in figure, figcaption instead of div for better semantics
    • Use rem instead of px since it's not scalable
    • Remove the height from .NFT-card, let the content dictates the height and use max-width instead of width
    • You don't need to add width: 275px on each section to prevent them from touching the edges, simply add a padding on the container to achieve that
    • For images you just need to add max-width: 100%; and display: block; and not a set width and height
    • I'd suggest you to add a css reset in your stylesheet to avoid any weird browser behaviors, i use this css reset

    Hope these help, Happy coding !!!

    Marked as helpful
    0
  • Adam Elitzur•110
    @AdamElitzurSubmitted almost 3 years ago

    Hi guys, for some reason, when the screen gets too small, the text doesn't wrap, and it just gets cut off. Any suggestions? It might be something with the margin, I'm just not sure what the best way is to do this. Thanks!

    Order Summary Card (HTML and CSS)

    2
    Heli0s•670
    @zeerobitPosted almost 3 years ago

    Good work completing this challenge.. The card size is too big for mobile view, add a max-width in your media query to make it smaller .

    1
  • Natasha Joy Vermeulen•20
    @natashajvandamSubmitted almost 3 years ago

    I ended up using a media query to adjust how two of the testimonials take up space in the grid. When the screen is wide, they span two columns, when the screen is small they span two rows. Is there a better way to use grid to naturally do this?

    Additional tips on how to make the responsive-ness more smooth are also welcome.

    Responsive CSS Grid Solution

    4
    Heli0s•670
    @zeerobitPosted almost 3 years ago

    Good work completing this challenge. Your grid template columns value is what's causing the issue you're having, here are a few pointers:

    • change grid-template-columns: repeat(auto-fit, minmax(240px, 1fr)); to `grid-template-columns: repeat (4, 1fr);
    • to center your grid properly remove margin: 8vw 0; from .grid then from your .content class change justify-content: space-between; to justify-content: center;
    • Add this to your media query to fix your layout for mobile:
    .grid {
    grid-template-columns: 1fr;
  }
  
  #Daniel {
    grid-column: 1;
  }
  
  #Patrick {
    grid-column: 1;
  }
  
   #Kira {
    order: 2;
  }
    • As suggested by Dev, to fix the accessibility issue, replace the div element for your content class to main, also wrap the attribution section in a footer
    • It'd be much easier to build this layout using grid area, here is a grid area tutorial check it out
    • use rem instead of px since px is not scalable
    • for this type of testimonial card design, the proper html semantics to use would be figure to wrap the content of each card, figcaption to wrap the names, title and blockquote to wrap the paragraphs. You can read more about it here
    • Replace the h4 tags to p since they're paragraphs and not headlines
    • Adding a css reset can prevent weird browser behavior, i use this one

    Hope you find these helpful, happy coding!!!

    1
  • Heli0s•670
    @zeerobitPosted almost 3 years ago

    Good work completing this challenge.. to get the exact sizing you need the pro subscription in order to get the sketch and figma files otherwise you can install a browser plugin called PerfectPixel which can help you with the sizes. While looking at your solution i noticed a couple of things:

    • To correctly center your card, remove top: 4rem; margin: auto; from the container then on the body selector add min-height: 100vh; display: grid; place-content: center; you will need to adjust the background image as well
    • no need to put a height on your container, let the content dictates the height
    • the hero image should be a background image and not added in the html
    • images should have max-width: 100%; instead of width also add display: block
    • your "payment-button" and "cancel-order" classes should be wrapped in anchor tag and not button
    • You should add a css reset to prevent any weird browser behaviors, i use this css reset
    • the card is too big for devices lower than 400px, you should add a media query

    Hope you find these feedback helpful, Happy coding !!!

    Marked as helpful
    2
  • Heli0s•670
    @zeerobitPosted almost 3 years ago

    Good work completing this challenge, it looks good. Here are a few pointers:

    • for this type of testimonial card design, the proper html semantics to use would be figure to wrap the content of each card, figcaption to wrap the names, title and blockquote to wrap the paragraphs. You can read more about it here
    • Replace the h1, h2 with the p element since they're paragraphs
    • use rem instead of px since px is not scalable
    • add a max-width to your container to keep the cards from stretching out too much on large screen

    Happy coding !!!

    Marked as helpful
    1
  • Tassie Oshiro•60
    @tassieoshiroSubmitted almost 3 years ago

    The part I have more difficulty with was trying to center the background waves on the center, but it was a fun project to build. Any advice on how to execute the project would be appreciated. Thanks!

    Card summary using sass

    5
    Heli0s•670
    @zeerobitPosted almost 3 years ago

    Good work completing this challenge.. however when viewing it on my laptop part of the card is cut off since i'm unable to scroll due to the overflow: hidden; property you added to the body. Adding this property to the body removes the scrolling bar which can cause issues when viewing this page on certain screen sizes such as my laptop .

    Happy coding !!!

    Marked as helpful
    0
  • Heli0s•670
    @zeerobitPosted almost 3 years ago

    Good work tackling and completing this challenge, here are a few pointers:

    • as @steevencode mentioned the mobile design is not responsive, you should revise your media query
    • no need to set a height on your ".testimonials-section" class
    • revise your grid properties for the mobile design, you should not have to span columns to 4 for mobile size below 375px or even above since it can't fit this many column
    • look up grid area it'd make it a lot easier for this challenge, check out this grid area tutorial
    • the report indicates you have many issues in your html since you use section tag it's expecting each section to have a header . The correct semantics for this challenge would be figure to wrap the content for each card, figcaption for the author section and blockquote to wrap the paragraphs
    • Nothing wrong with a desktop first approach design but I usually recommend mobile first since it makes it a lot easier to design and it can save you a lot on css

    Hope this helps, happy coding !!!

    Marked as helpful
    1
  • Carlos Tudeich•80
    @CarlosTudeichSubmitted almost 3 years ago

    Hi! thank you for seeing my soulution. I'm open, and please tell me how to do it better!

    NFT preview

    2
    Heli0s•670
    @zeerobitPosted almost 3 years ago

    Good work completing this challenge, couple of pointers:

    • you should use rem instead of px since px is not scalable
    • wrap the equilibrium image in an anchor since it's supposed to be an interactive element
    • A simpler way to position the icon-view image without using any margin or pseudo element, same for the background-color would be to add a div in the html after the ".img-container" you could name it ".overlay" then add the icon-view image inside of it also wrapped in a anchor tag for interactivity. So in your css you could set the width and height to 100% for the ".overlay" div then use position absolute to place it on top of the ".image-container". Add the background-color in rgb so you can add the opacity value, it should be like this background-color: rgb(0, 255, 247, 0.4); hence 0.4 is the opacity value, then use flexbox on the ".overlay" to center the icon-view image . Add opacity: 0 in the overlay div to make the background-color and the icon-view invisible then for the hover effect add 'opacity: 1`

    Hope this helps, Happy coding !!!

    Marked as helpful
    0
  • Heli0s•670
    @zeerobitPosted almost 3 years ago

    Good work completing this challenge, it looks good. I'd also like to point out that this challenge should be done with css grid so maybe you can come back to it later once you learn grid . By the way, i noticed you uploaded the default readme file in your repo, this readme file simply contains instructions/guidelines on how to approach this challenge so you should either rename it or move it to another folder. The README-template.md is the one you need to edit then rename it to README.md to upload to your repo.

    Happy coding !!!!

    1