Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found

All comments

  • Nikola 190

    @porumbachanov

    Posted

    Hi there, I have a few suggestions that could improve the design.

    1. The form is way too stretched, I would suggest using "rem" instead of percentage for the "max-width".
    2. For the custom list image (the checkmarks), instead of targeting the "ul" with the "::before" pseudo-element, you can directly change the list style of the "ul" by using list-style-image: url(url to image).
    3. There really is no need for using CSS Grid, all of this could be accomplished with Flexbox, it would make the code look way cleaner. You can accomplish this by separating the content (the text, form, button, etc.), and the image into two "container" divs and just display: flex them.

    Not sure if the last point made sense, but hope it this helps.

    0
  • Nikola 190

    @porumbachanov

    Posted

    Hi there, I noticed that the responsiveness breaks at 712px. I would suggest changing the flex direction to column instead of row so that it doesn't look weird. Also I see you are using absolute units (px), it is best practice to use relative units like rem to avoid sizing issues on different screens, also it is the standard for responsive design. Hope this helps!

    Marked as helpful

    0
  • Nikola 190

    @porumbachanov

    Posted

    Hi there, your responsiveness breaks on tablet sized screens, the image reaches its maximum width and can't take up more space. This is easily fixable if you apply max-width to the parent container. The background color is not the same as in the design, the colors are all described in the style-guide.md file. Hope this helps :)

    Marked as helpful

    0
  • @induwara-thisarindu

    Submitted

    What are you most proud of, and what would you do differently next time?

    Hi ✌️ I made this project with HTML and CSS and used mainly flexbox

    What I am proud of is that I was able to finish this under 1and half hour almost without much trouble

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

    What I had trouble is when I styled the perfume I did not really have an idea on how to do it so I ended up using span hope I am right would love a review on this 😊

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

    I would like help with how to style the perfume text line I used span to it is there any better way?

    Thanks 😊

    Nikola 190

    @porumbachanov

    Posted

    Hi, in response to your question you asked on my solution, I'm not sure I can give you proper feedback, however I can tell you what I did.

    Basically what I did was have one parent div in which I have two children divs, one for the image and one for the text content. I set the container to 45rem (for desktop) and the height to 100%, that way I let the content inside the parent container dictate the height. And of course I'm using flexbox for the layout. So basically for the whole size thing, I just have a fixed width for desktop and mobile and the height is dictated by the content, so the margins, font sizes, etc all play a part in the height.

    Dunno if I made sense but hope this helps.

    P.S. You can style the spacing in the "perfume" text without using <span> by using letter-spacing: :)

    Marked as helpful

    1
  • Nikola 190

    @porumbachanov

    Posted

    Looks good!

    0
  • @ARUNKUMAR2906

    Submitted

    What are you most proud of, and what would you do differently next time?

    none

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

    none

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

    none

    Nikola 190

    @porumbachanov

    Posted

    Hi, nice work! I have a couple of suggestions that could improve the visual appeal.

    1. You can add some transitions when switching from desktop to mobile view so that it doesn't clip.

    2. The text is a different color than that show in the design, changing that color to the lighter grey color in the design will make things look much better.

    3. The numbers in the ordered list are different from the design as well, you can color them with the given color by utilizing the ::marker pseudo element like so:

    .instructions ol li::marker{color: ...}
    
    0
  • Louise 130

    @atheenaoteyza

    Submitted

    What are you most proud of, and what would you do differently next time?

    .

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

    .

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

    .

    Nikola 190

    @porumbachanov

    Posted

    There is no need for the margin: 18% auto; in the .container as it needlessly stretches the window. Instead, you can center the items using flexbox by adding the following lines in the "body":

      display: flex;
      flex-direction: column;
      align-items: center;
      justify-content: center;
      height: 100vh; 
    

    display: flex; Enables the flexbox layout.

    flex-direction: column; Specifies the flex container's main axis to column, which means that the child elements will be laid out vertically.

    align-items: center; Centers the items horizontally.

    justify-content: center; Centers the items vertically within the container.

    height: 100vh; Sets the height of the "body" element to occupy the full height of the viewport ("vh" stands for viewport height).

    Hope this helps :).

    Marked as helpful

    1
  • @marijapavic

    Submitted

    What are you most proud of, and what would you do differently next time?

    .

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

    .

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

    .

    Nikola 190

    @porumbachanov

    Posted

    Cool, you can try centering the card. I can't give proper feedback, because I can't open your code or the live site.

    0
  • sniiii 20

    @sniiii

    Submitted

    What are you most proud of, and what would you do differently next time?

    Didn't take me long time at all, and haven't written css from scratch before

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

    First made the CSS for desktop, wasn't sure how to do it well for mobile but think i figured it out?

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

    Appreciate feedback on everything!

    Nikola 190

    @porumbachanov

    Posted

    1. You are already using flexbox and relative units, so there is no need for media queries as it just breaks the layout on the screens between the ones defined (between 750px and 600px).

    2. For the border-radius, it's probably more appropriate to use absolute units like "px" because it kinda looks weird with percentages.

    P.S. Hope this helps. I'm not very experienced, but I have to write a feedback to progress on the learning path I started. xD

    0