@AllRedCatSubmitted 12 months ago
What did I can impruve in my CSS skills
Patrick Bryan
@pbryan9All comments
- @pbryan9Posted 12 months ago
Just about perfect on desktop! The only improvement I can spot there is to do with font sizing and padding -- your elements are crowding the edge of the card just a little bit on desktop.
Mobile looks good at the ideal 375px size, but it should probably keep the column layout for another couple hundred pixels -- the larger layout doesn't quite fit until the viewport gets a little over 600px.
Nice job!!
Marked as helpful1 - @SyuusukeFujiSubmitted 12 months ago
I had a lot of issues with this project, I struggled a lot placing the pictures around and let's not talk about the accordion's moving elements around.
All feedback is welcome!
@pbryan9Posted 12 months agoGood job SyuusukeFuji!
I feel ya, images are painful. Sometimes it can help to stick the image into a wrapper (div or similar) and give the img a width of 100% -- that way you can control the size a little more easily by styling the wrapper.
The box image in particular lends itself to a pattern that I reach for over and over again since it wants to sort of straddle the edge of its container. One reliable way to do that is to set the image's parent element to
position: relative;. Then, make the imageposition: absolute; left: 0;. That'll make it bump right up against the left edge of the container; to get it to split the line, you can add atransform: translateX(-50%);to the image and it'll wind up right where you're wanting it (this pattern works just the same for right, bottom, whichever - just get the translateX / Y and the +/-50% set correctly).On the other illustration, I think there's an issue with the background-position property...it seems to behave a little bit better when I take that away completely, so toning down the values might get it to sit closer to where you're wanting it. It most likely needs to be positioned by its left edge.
Keep up the good work!
Marked as helpful0 - @aabc24Submitted 12 months ago
Please tell me how to improve my code...
@pbryan9Posted 12 months agoI think it's pretty solid, nice work!
I came up with just a couple small notes:
- I'd say some of the sizing is a little bit off compared to the design (see for example the font size on the input and the padding on the desktop success message).
- The validation is good but I might argue it's a bit too eager - you might consider triggering input validation on blur / submit...as it's currently written, it goes into error mode with the first character typed.
Marked as helpful1 - @pbryan9Submitted 12 months ago@pbryan9Posted 12 months ago
Had a bit of difficulty sizing the image for the desktop view...wound up using
background-size: coverand rolling my own border radius (rather than relying on what was built into the image), but I'd be interested to hear other approaches. Images have generally been painful for me XDThanks for looking!
0 - @chrisdh80Submitted 12 months ago
Hi all, for the first time I tried to use @media. One problem I encountered was how to remove the border radius (at the bottom of sedan and top of luxury) when it switches from full screen to mobile view. Additionally any other feedback regarding the responsive element would be helpful, as im just starting to learn this.
Additionally I set the colors as variables, thanks to some very helpful feedback. It was going well until I tried to use it on the background of the flex items(.sedan, .suv and . luxury). When I applied the var all the colors disappeared. im not quite sure what i did wrong, so any feedback would be great.
@pbryan9Posted 12 months agoCongrats on breaking into
@mediaqueries! It's a huge & important part of css -- you'll learn to love 'em.With regards to the border radius problem you encountered, did you know that you can set the radius for each corner independently? Usually we want to set all 4 corners the same so we use the
border-radius: 12px;shorthand (single-value), but you can instead provide 4 values for the same property to specify each corner in clockwise order beginning from the upper left. As an example, you can sayborder-radius: 10px 0 0 10px;to get rounded corners on the left side while keeping squared corners on the right side.I didn't dig in to prove exactly what's causing the colors-via-variables issue you described, but I do see a couple of things likely to contribute to the problem:
- When you make use of the variables, they need to be capitalized in exactly the same way as they were declared. In other words,
--Bright-orangeis not the same as--bright-orange(call 'em what you want, but usual preference is to use the lowercase versions - these are more conventional). - When declaring the variables within the :root selector (or anywhere else really), you should end each line with a semicolon
A couple other quick notes:
- The design calls for each of the headings to be uppercase
- It looks like the body text has the default browser font instead of the specified Lexend Deca one
Good luck! CSS is a surprisingly deep language, and it can be a lot of fun.
0 - When you make use of the variables, they need to be capitalized in exactly the same way as they were declared. In other words,
- @yassawambessaw24Submitted 12 months ago
If anyone can point out an error, please let me know that's how i'll learn. Thank you in advance. xoxo
@pbryan9Posted 12 months agoNice work!!
There are a couple of items that will get this a little closer to the design jpg:
- The background colors of each of the 4 score cards on the right should match their text color (yours appear to use green on each).
- Similar minor point on the background color: doesn't quite line up to the spec.
- The contents of each of the cards could use a little breathing room: notice how the score crowds a little close to the edge on the right-hand side of its container (same with the icon on the left side).
- Finally, I think your shadow looks great as-is, but if you want to make it look more like the design you can increase the spread on your box-shadow property.
Happy styling!
Marked as helpful0
