Skip to content
OverviewSolutions
12
Comments
8
MURRAY122
@MURRAY122

All comments

  • MURRAY122ā€¢280
    @MURRAY122Posted over 2 years ago

    Hi Atharva Hinge,

    Congrats on your first solution! I had a quick look at the code and think this might help.

    • Use main tag to indicate a landmark. This helps with the overall structure of the HTML and improves accessibility.
    <body>
<main></main>
<footer></footer>
</body>
    • You should also think about your heading levels, these should go in order from h1 to h6.
    • Your alt text for the image shouldn't contain the words 'picture', 'photo' or 'image'. Perhaps just the name of the parfum and the word bottle would do.

    Hope these might help for you.

    Marked as helpful
    0
  • MURRAY122ā€¢280
    @MURRAY122Posted over 2 years ago

    Hi Kevin Tatagiba,

    Congrats on the challenge, I seen some of your code and think this could help.

    • Your html needs landmarks which help with accessibility.
    • Instead of using position: relative on your picture and div elements and setting their fixed positions like top, you could try wrapping your picture and div box within <main></main> and then following could style them
    body{
   height:100vh; //add to your body styles
   margin: 0; // add to your body styles
}
main{
   display: flex;
   flex-direction: row;
}

    This could then center your main element to the center of the screen.

    0
  • MURRAY122ā€¢280
    @MURRAY122Posted over 2 years ago

    Hi Pravallika Reddy,

    Congrats on the challenge! Just a quick couple of things that might help.

    • Consider using a separate file for your CSS called "styles". You can then link your styles to your html page with <link rel="stylesheet" href="styles.css"> within your head tag.
    • Also consider using a media query that way you can change styles of elements such as their padding, margins, or font sizes etc... depending on the user's screen size.
    • Your img tag would be best suited in a div tag as h tags are meant for text.

    Congrats again on the challenge. Keep it going!

    0
  • MURRAY122ā€¢280
    @MURRAY122Posted over 2 years ago

    Hi Radek,

    Congrats on your first design! I took a quick look, and it seems a bit large for the screen sizes. Seems like there is a lot of set margins and padding for elements. You might want to consider the following to start with:

    body{
    display: flex;
    align-items: center;
    justify-content: center;
    height: 100vh;
    width: 100vw;
}

    You could then remove the width and margin styles on main to center the element. Then you could set the img to height: 450px and remove its width. From there it would be about changing the products description and title font sizing, padding and margins etc...

    • You may also be interested in media query to help support responsive design and answer any question you might have had with it.
    Marked as helpful
    0
  • MURRAY122ā€¢280
    @MURRAY122Posted over 2 years ago

    Hi Yonas,

    Congrats on the challenge! I had a quick look at the code and have some tips for you.

    • Your container-submit is hidden behind your container. Perhaps using a custom class such as .hide { display: none } would be more suitable? Then once the user has hit submit you could use JavaScript to remove the hide class from container-submit and apply it to your first container. You can learn more on how to add and remove classes with JavaScript.
    • I also noticed that on the mobile view the first container seems too large. You could use media queries to change the style of that class. For example..
    @media screen and (max-width: 375px) {
  .container {
    width: 0;
    height: 0;
  }
}

    Any classes within that media query will be applied when the users screen is no more than 375px wide. You could also change it to min-width: 375px which means any classes within the media query will apply when the users screen is more than 375px, depending on your preference.

    Marked as helpful
    1
  • MURRAY122ā€¢280
    @MURRAY122Posted over 2 years ago

    Hi,

    Good solution! In terms of accessibility, it would be best to implement landmarks (you can click view report button above the comments on this page to view more) to make the structure of the page to be more easily understood for things like screen readers etc..

    A basic example would be to use the main landmark within your body tag. <body><main></main><body>

    you can also read more about it here (W3)

    Marked as helpful
    0
  • MURRAY122ā€¢280
    @MURRAY122Posted over 2 years ago

    Hi, Taking a quick look at the code, seems your svg icon-cart is not displaying on the live site. The src your calling would work in your development environment but fails to find it in production. Changing it similarly to how you have displayed your product image would correct it. ./images/icon-cart.svg. The svg tag could also be within the DOM itself if that also suits

    In terms of helping users view it on different screen sizes I noticed you had @media screen and (max-width: 370px) { } With max-width set to 370px, anything within this media block will display when the users screen/view is no more than 370px. Try increase the pixels to say 780px for example and you will see what I mean.

    • W3schools SVG
    • W3schools helped me out with responsive design so it might interest you.
    • There is also a resource tab on frontend mentor with lots of different Info
    0
  • Rich Kyle Baguiā€¢20
    @richbaguiSubmitted over 2 years ago

    I find it difficult to be responsive, it is only a web page and not a mobile-friendly page. How can I learn more about CSS and do I need to learn framework?

    Product Preview Card

    1
    MURRAY122ā€¢280
    @MURRAY122Posted over 2 years ago

    Hi, Rich Solution is looking good! Regarding learning more about responsive web design, you could look here for a quick responsive web design Introduction by W3Schools. Hopefully, it will help answer some more questions you might have and is a good starting point to begin with.

    1