Skip to content
OverviewSolutions
5
Comments
11

Thomas Burette

@tburette190 points

@tburette

Latest solutions

Latest comments

  • Thomas Burette•190
    @tburette

    Nice result and nice code!

    You use BEM. In your header there are classes for the modifiers but not the block. You have class="header__title but no class="header".

    To do the layout of the cards in the desktop layout you used a grid with three columns and four rows. What I did was that I had three columns and two rows. First and last card span the two rows. Middle two cards occupy their own row (+ some adjust toward the center). I find your solution simpler than mine.

    I also noticed that to have an item span one column you used grid-column-start: 1;. I didn't you could do that. I'm used to do grid-column: 1 / span 1;.

    I'm curious. Where does the css reset that you use come from?

  • Thomas Burette•190
    @tburette

    Nice solution @IamVictheCG !

    Here are a few remarks.

    Your html could be more semantic by using <main>, <section>, ...

    I find your word spacing on the titles a bit too big but that is a purely stylistic choice.

    I see that you use separate css files for desktop and mobile. Interesting. Did you know that you can import css files conditionally?

    css: @import url(a.css) (min-width: 800px);

    html: <link rel="stylesheet" href="a.css" media="(min-width: 800px)">

  • Thomas Burette•190
    @tburette

    Nice solution. The CSS is simple to read and does the job in few lines!

    Instead of .btn:hover{} you could do .btn:hover, .btn:focus. It is better for accessibility : it makes the CSS works if people use keyboard navigation (tabbing).

    You could use more semantic elements in your HTML : <main>, <footer>, <section>,...

  • Thomas Burette•190
    @tburette

    Nice solution!

    Your CSS could follow BEM more. You did <div class="card sedans"> but you could have done <div class="card card--sedans">. See Why the modifier CSS classes are not represented as a combined selector?

    When the button becomes active the card moves a little. Is it intentional? It could be a neat effect. If it is not : you add a border on :hover/:focus. It causes the layout to change.

    Marked as helpful
  • Thomas Burette•190
    @tburette

    The result looks nice. It doesn't exactly match the given design and that's okay! (Adjusting a layout to closely match the given challenge is a lot of time for little payoff.)

    In the HTML:

    • There is an unclosed HTML comment at the end
    • <div class="attribution"> could be a <footer
    • you used an <h3> for the link/button. Isn't there an HTML that would be a better fit?

    In the CSS:

    • When the viewport is just above 830px part of the content is not visible and there is horizontal scrolling. There seems to be some kind of mixup in your media queries (@media).
    • You could implement the effect on the button when it is active (see design/active-states.jpg)
    Marked as helpful
  • Thomas Burette•190
    @tburette

    Nice solution. You used place-content which I didn't know about. It is a shorthand property for align-content and justify-content.

    There is a smooth transition when the breakpoint is it : the vertical space between the items within each of the three cards smoothly increase/decrease. I looked at the code and it is caused by :

    .button {
  transition: all 0.4s;
}

    At first I didn't understand why transition: on a button affect the space between the items of a grid. By fiddling around I realized it is the margin-top property of the button which transitions.

    There is a bunch of CSS that is unused in the final solution : in .button {} there are two border:. .orange{}, .green{}, .cyan{} are unused,..

    Marked as helpful
View more comments