Skip to content
OverviewSolutions
15
Comments
4
Rachid Ben
@rachidbend

All comments

  • Rachid Ben•410
    @rachidbend

    I love how you made it functional as if it's part of a larger website, and the 'Pages are under development' touch. It looks so close to the design in the desktop view, well done. I have two small nitpicks, when you hover over the large image, there is no indication that it's clickable, maybe add a 'cursor: pointer' to it, and in the mobile view you restrict the height of the images, which warps them, a suggestion is to allow them to grow in height as well to keep the same aspect ratio. You did a splendid job, keep going, and good luck. 😁

  • Rachid Ben•410
    @rachidbend

    Hi, great work by the way, in the start it can be difficult to know how you can do something but we still have to try.

    I think a great thing to learn is how to separate a design into multiple components which can help quite a bit in styling it, for example, you put the image everything in a box because that part can be a component, but instead of writing the rest into a paragraph, you could have split them, you could have put 'PERFUME' into an <h3> or another paragraph by itself, the put the title below it into an <h1> tag, and the paragraph into a <p> element instead of putting them all in a single <p> element and splitting them with <span> and <br>, you would have not needed them anyways. basically try to think of what are the parts in a design, and if they should be put into a single tag (like how the image and the rest of the text are inside the box element) or separated into multiple tags (like how the title should be in a <h1> tag by itself).

    i see that you wrote two different versions in the HTML, but here's another way you could have implemented it using CSS grid, inside the box element, put the image inside a <div>, and the rest of the text into another div next to the first, and use CSS grid on the box (display: grid) and (grid-template-columns: 1fr 1fr) so that it each would take half of the box, and in the media query, bellow 700px for example or wherever you want to see it, add this style on the box (grid-template-columns: 1fr)(grid-template-rows: 1fr 1fr) to change their placements, then you wouldn't need to use HTML for the mobile, you can do it in CSS only.

    sorry if this was too much or unclear, and if you want to see a code example of what I said I can provide it. good luck.

    Marked as helpful
  • Rachid Ben•410
    @rachidbend

    looks like it's working properly, well done. But just as a tip, use Figma or Adobe XD, put the design image in it, and use it to figure out the font sizes, margins, heights, widths, gaps, line heights and anything else you may need, it can be quite helpful in getting your website looking as close to the original design as possible, but if you can't get them to match perfectly it's ok, just get them as close as possible, other than that you're doing great, keep going and good luck in your journey.

    Marked as helpful
  • Rachid Ben•410
    @rachidbend

    when you reset or load the page the first time, the numbers in the bill and the number of people should not be set from the start, they should be empty, to not let the user have to change them, but let them put the numbers themselves. other than that, Great job.

    Marked as helpful