Skip to content
OverviewSolutions
7
Comments
9
Cagri Kizilkan
@cgrkzlkn

All comments

  • Sudharshan S•90
    @Sudharshan-24Submitted about 2 years ago

    It is an interesting challenge I loved it a lot by doing this challenge. However, I am unable to make the mobile images responsive manner.

    Solved image issue for mobile view :)

    Product Preview Card Component - Responsive

    1
    Cagri Kizilkan•210
    @cgrkzlknPosted about 2 years ago

    Great job! You can use picture and source elements for responsive images:

    <picture>
  <source
    srcset="./images/image-product-mobile.jpg"
    media="(max-width: 375px)"
  />
  <img class="perfume-image" src="./images/image-product-desktop.jpg" alt="Perfume"/>
</picture>
    0
  • Akshay Kumar Mondal•140
    @akshaykumarmondalSubmitted about 2 years ago

    I am finding difficulty regarding the setting of image according to media query. Its running in my browser but when i am trying to run through the repository it is showing the value of alt attribute of image tag instead of image

    ProductCard

    1
    Cagri Kizilkan•210
    @cgrkzlknPosted about 2 years ago

    Great job! I see the issue you mentioned, it is because src attributes in your img elements.

    Update them as follows:

    src="/images/image-product-desktop.jpg" to src="./images/image-product-desktop.jpg"

    src="/images/image-product-mobile.jpg" to src="./images/image-product-mobile.jpg"

    Marked as helpful
    0
  • Rita•40
    @ritaogadaSubmitted over 2 years ago

    How do I resize the overlay view-icon image?

    Thanks! :-)

    NFT card component

    1
    Cagri Kizilkan•210
    @cgrkzlknPosted over 2 years ago

    Great job! Icon size problem is because of this CSS:

    .equilibrium-pic img {
   width:100%;
}

    Remove this CSS and also remove width:50% on the view icon.

    Then add a class to the main image:

    <img class="main-image" src="./images/image-equilibrium.jpg" alt="equilibrium-image">

    And add that CSS only to this class:

    .main-image {
   width:100%;
}
    Marked as helpful
    1
  • Carmen•490
    @CarmenyoSubmitted over 2 years ago
    What are you most proud of, and what would you do differently next time?

    Approaching React through this simple project has allowed me to gain a clearer understanding of what happens under the hood when I implement React.

    What challenges did you encounter, and how did you overcome them?

    The challenge was organizing my files and gaining a clearer understanding of how React works.

    What specific areas of your project would you like help with?

    In this project, I haven't needed help nor had significant doubts

    Product Preview Card Component

    3
    Cagri Kizilkan•210
    @cgrkzlknPosted over 2 years ago

    Great job! I have a suggestion to improve your code:

    • Instead of using 2 img tags on your HTML, you can use a picture tag to switch between desktop and mobile images. It doesn't even require CSS, only HTML:
    <picture class="image-wrapper">
  <source
    srcset="./images/image-product-desktop.jpg"
    media="(min-width: 768px)"
  />
  <img src="./images/image-product-mobile.jpg" alt="Frasco Perfume"/>
</picture>
    Marked as helpful
    0
  • Arbaaz Khan•50
    @arbaaz99Submitted over 2 years ago

    Q1. What did you find difficult while building the project? Ans. Understanding flexbox and media queries were a bit difficult for me.

    Q2. Which areas of your code are you unsure of? Ans. Responsiveness.

    Q3. Do you have any questions about best practices? Ans. As this is my first project, I don't know much about anything. Any suggestions of you will be helpful

    Product Preview Cart

    1
    Cagri Kizilkan•210
    @cgrkzlknPosted over 2 years ago

    Great job! I have several improvement suggestions:

    • Image looks blurry in your page, you can fix it by adding object-fit:cover; . Like this:
    .main-fol .image {
    object-fit:cover;
}
    • I see that you only use image-product-desktop.jpg. It would be better switch to image-product-mobile.jpg on mobile screens. You can do it like this:
    <picture class="image-wrapper">
  <source
    srcset="./images/image-product-mobile.jpg"
    media="(max-width: 475px)"
  />
  <img src="./images/image-product-desktop.jpg" alt="Perfume"/>
</picture>
    • You can use letter-spacing CSS property for 'PERFUME' word.
      • Change <p class="start">P E R F U M E</p> to <p class="start">PERFUME</p> And add this CSS:
    .start{
   letter-spacing:4px;
}
    Marked as helpful
    0
  • Cagri Kizilkan•210
    @cgrkzlknPosted over 2 years ago

    Great job! I have several improvement ideas:

    • You can use HTML5 Landmarks for accessibility. HTML5 Landmarks
    • You can change <div class="qrcode"> to <main class="qrcode">
    • You can wrap <div class="attribution"> with a footer tag. Like this:
    <footer>
   <div class="attribution">
   </div>
</footer>

    .qrcode{
   box-shadow: 0px 10px 20px 0px rgba(0, 0, 0, 0.1);
}
    Marked as helpful
    0
  • Chris Ekstedt•110
    @cekstedtSubmitted over 2 years ago

    Are there any best practices, in semantics or styling, that you would suggest as an improvement to my code?

    QR Code Component

    2
    Cagri Kizilkan•210
    @cgrkzlknPosted over 2 years ago

    Great job! You can add box shadow to the card element. I think that will be a nice improvement :)

    You can use this tool to generate box-shadow: CSSmatic Box Shadow Generator

    For example :

    .card{
   box-shadow: 0px 10px 20px 0px rgba(0, 0, 0, 0.1);
}
    Marked as helpful
    0
  • Asther Marie Moreno•30
    @AshM10Submitted over 2 years ago

    What I learned

    Sometimes I get really nervous writing code for no apparent reason! I felt the same way here but doing a step by step procedure on how I want to tackle the project helps a lot for me. By breaking down the project into smaller steps, it got easier for me and I felt less overwhelmed.

    Responsive QR code component Landing Page

    1
    Cagri Kizilkan•210
    @cgrkzlknPosted over 2 years ago

    Good job! Make sure to use landmark elements for accessibility. HTML5 Landmarks

    • You can change your <div class="container"> to <main class="container">
    • You can wrap <div class="attribution"> with footer. Like this:
    <footer>
   <div class="attribution">
   </div>
</footer>
    Marked as helpful
    1
  • Victory Q•30
    @vquiahSubmitted over 2 years ago

    This challenge was completed using HTML and CSS. I had trouble with determining how many containers I wanted to use but figured something out in the end. What are everyone's thoughts?

    QR code component

    1
    Cagri Kizilkan•210
    @cgrkzlknPosted over 2 years ago

    Good job! Yes, 1 container is enough for this project. Also make sure to use landmark elements for accessibility. HTML5 Landmarks

    • You can change your <div class="container"> to <main class="container">
    • You can remove unnecessary Font Awesome script on your HTML.
    • You can add box shadow to container element, this tool can be useful : CSSmatic Box Shadow Generator
    0