Added animations on mobile to simulate the hover styles on desktop
What specific areas of your project would you like help with?Code optimization and use of a system
Added animations on mobile to simulate the hover styles on desktop
What specific areas of your project would you like help with?Code optimization and use of a system
Hi there, I have a few suggestions that could improve the design.
"rem"
instead of percentage for the "max-width"
."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)
.display: flex
them.Not sure if the last point made sense, but hope it this helps.
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!
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 :)
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 😊
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:
:)
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
Hi, nice work! I have a couple of suggestions that could improve the visual appeal.
You can add some transitions when switching from desktop to mobile view so that it doesn't clip.
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.
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: ...}
.
What challenges did you encounter, and how did you overcome them?.
What specific areas of your project would you like help with?.
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 :).
.
What challenges did you encounter, and how did you overcome them?.
What specific areas of your project would you like help with?.
Cool, you can try centering the card. I can't give proper feedback, because I can't open your code or the live site.
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!
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).
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