Skip to content
OverviewSolutions
7
Comments
6
Patrick Bryan
@pbryan9

All comments

  • Patrick Bryan•180
    @pbryan9Posted 11 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 helpful
    1
  • SyuusukeFuji•170
    @SyuusukeFujiSubmitted 11 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!

    FAQ Accordion with Flexbox

    1
    Patrick Bryan•180
    @pbryan9Posted 11 months ago

    Good 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 image position: 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 a transform: 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 helpful
    0
  • Patrick Bryan•180
    @pbryan9Posted 11 months ago

    I 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 helpful
    1
  • Patrick Bryan•180
    @pbryan9Posted 11 months ago

    Had a bit of difficulty sizing the image for the desktop view...wound up using background-size: cover and 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 XD

    Thanks for looking!

    0
  • chrisdh80•70
    @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.

    3 column preview card component using HTML and CSS

    1
    Patrick Bryan•180
    @pbryan9Posted 12 months ago

    Congrats on breaking into @media queries! 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 say border-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-orange is 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
  • Patrick Bryan•180
    @pbryan9Posted 12 months ago

    Nice 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 helpful
    0