Skip to content
OverviewSolutions
30
Comments
9
Praveen Perumal
@solvedbiscuit71

All comments

  • Praveen Perumal•740
    @solvedbiscuit71

    @laachouch8, You solution looks great 😄

    I have some tips to improve it,

    • Firstly, wrap everything in <body> with <main> tag to avoid accessibility issue
    • Gallery is taking the size of the page instead of the size of the screen, it would be better since, user don't need to scroll down to change the image.
    • And no of items in cart should go till zero, and then you can disable it.

    These are some tips i have for you, hope it helps

    Marked as helpful
  • Praveen Perumal•740
    @solvedbiscuit71

    Hi, Jonathan

    No, i don't think there is another way around it. since, the background image are big we are forced to use background-size and background-position to position accordingly.

    But, it would be better if you use viewport vw units instead of px so it will be more responsive! Check out: my solution

    Hope that resolves your question.

    Marked as helpful
  • Praveen Perumal•740
    @solvedbiscuit71

    Hi Athira, Good work on your first angular project in frontendmentor!

    I have some tips to improve your code,

    • wrap everything inside the body with a <main> tag to avoid accessibility issue.
    • use <div> tag instead of <span> for element with class "detail","eth","time"
    • use display: flex with align-items: center in "eth","time" and footer to vertically center the image and text.
    • add the hover state for <span class="name"> element.
    • add min-height: 100vh; in <main> tag to vertically center the card.

    Addressing the use <div> instead of <span>, we generally use <span> as a "leaf node" means that it doesn't have other childNode except the textNode. it's a best practice

    Hopefully you find these suggestion helpful 😊

  • Praveen Perumal•740
    @solvedbiscuit71

    Hi Nikhil, good work in your first challenge.

    I have some suggestion which might be helpful,

    1. Change <p id="p1"> to <h1> and <p id="p2"> to <p> to simplify and avoid accessiblity issue!
    2. For your text align issue
    .container {
  display: flex;
  flex-direction: column;
  gap: 1.5em;
}

    Here, we are changing the flex direction to "column" i.e vertically and adding a gap of "1.5em" (or some else) between flex items.

    Then, remove display: inline-block; & position: fixed in p#p1 & p#p2 and adjust margin & padding accordingly.

    Happy Coding and Good luck on your frontend journey

  • Praveen Perumal•740
    @solvedbiscuit71

    Hi Andy, I used radial-gradient() for the background which looks close to the design

    .card {
    background-image: radial-gradient(circle at top,hsl(210, 19%, 18%) 0%,hsl(215, 23%, 14%) 50%, hsl(215, 27%, 12%) 100%);
}

    Check: https://www.frontendmentor.io/solutions/interactive-rating-component-with-vanilla-js-sass-3UZIEGY9H

    Hopefully, that solves your query.

    Marked as helpful
  • Praveen Perumal•740
    @solvedbiscuit71

    As @besttlookk mentioned, The white space at the bottom of the screen is because of the content of the page (height required) is less than the screen's height or viewport's height.

    We can resolve this issue by adding a min-height: 100vh; to the <section class="section"> tag and overall your design and transition between theme looks good!

    Marked as helpful
  • Praveen Perumal•740
    @solvedbiscuit71

    Hi there, good work on your solution

    Your solution is almost perfect but there are some issue in your code,

    1. There no need for a box-shadow on the .body-card as there is a visual difference between the background color and the card's bg color.
    2. Instead of using margin-top: 40px on .body-card use display: grid; with place-content: center; on the body will automatically center it.

    NOTE: set min-height: 100vh on the body for the (2) feedback to take effect.

    Marked as helpful
  • Praveen Perumal•740
    @solvedbiscuit71

    Nice work on your solution! Regarding the accessibility issue, change the <div class="attribution"> to <footer class="attribution"> with corresponding closing tags will resolve it.

    Overall, Good work and Gook luck on your frontend journey.

  • Praveen Perumal•740
    @solvedbiscuit71

    Hi Archerking47, Good work on your project.

    No, you need not use media queries or max-width on the container since here the card have fixed dimension so just using display: grid on the parent container with place-content: center will be fine.

    And regarding the accessibility issue wrapping the card element with a main tag will resolve it.

    Marked as helpful