Skip to content
OverviewSolutions
5
Comments
32

Joran Minjon

@DrKlonkUtrecht610 points

Developing myself from back-end to front-end. Here to get some needed practice!

Latest solutions

Latest comments

  • Joran Minjon•610
    @DrKlonk

    Hi Luka, nice to see you on the platform!

    Some (unordered) things that stick out to me:

    The centering doesn't really work on smaller screens, as Simone pointed out. Use margin auto, flexbox or grid for this to keep it nice and responsive.

    It seems like the responsive.css is the same as style.css. No need to repeat everything in a media query, just the things that are changing.

    The border radius of the card seems a bit off, I think it can be just one value.

    Using rem instead of px is usually preferred, because it increases responsiveness and thereby accessibility.

    Naming classes is hard, but can definitely be improved here. Try to come up with something that describes the content of the element. "Annual-plan" is a decent name in that sense, "div-button" is not. You might want to call that "button-group". Also "rand-text" suggests the text to be random, which it is not.

    Naming things is super important, even more so when collaborating with others.

    The annual-plan class can be centered more cleanly by removing max-width and left properties and introducing margin: 0 2rem or something similar

    Marked as helpful
  • Joran Minjon•610
    @DrKlonk

    Hi Maja,

    I agree with the others that CSS Flexbox is a great way to accomplish flexible layouts. I'd like to add Flexbox Froggy as an additional resource to practice with it!

    Happy coding!

    Cheers, Joran

  • Joran Minjon•610
    @DrKlonk

    Hi Agata,

    Nice solution! Good to read (in your readme) that using a framework would do nicely here, especially with the reusable components. I fully agree! It is well worth picking up one of them.

    I especially like the visually hidden headers on sections for our visually impaired visitors.

    Well done and happy coding! Cheers, Joran

  • Joran Minjon•610
    @DrKlonk

    Hi Jeremy,

    Nice job overall! Some things I've noticed:

    The third rating box should be: justify-content: flex-end (instead of end).

    The stars wrap from 640-653 pixels wide for the middle rating, that looks a bit weird. Also, from 792 to 426 pixels wide the stars seem to move upon resizing. They don't align properly with the other ratings either.

    All this is because the rating__stars resizes because of the flex-shrink, but the stars have a fixed width. I think it is better to remove the flex-shrink from this class.

    Btw, flex: 0 1 auto is the default, so you don't even have to set the growth and shrink explicitly!

    Cheers, Joran

  • Joran Minjon•610
    @DrKlonk

    Hi Raymart!

    Reaaaaaally well done on this one. Looks good, works good. A nice example for everyone to follow.

    Particularly, I like the little animation on the 'rules' text and the fact that you don't get points reducted when you lose.

    A minor thing I found in the scss: I think technically the flex classes like flex--j-between are utility classes and not variables. I'd expect a _variables.scss to contain only scss variables. But maybe that's just me. You could check out the 7-1 pattern to structure SCSS. I really like it.

    Just one minor thing I found in the code: in displayResult() the winner always gets set to playerChoice, although I don't think it affects anything.

    Again, very well done!

    Cheers, Joran

  • Joran Minjon•610
    @DrKlonk

    Hi Zach!

    Nice job for your first challenge! Don't be misled by the word newbie for this one, it is quite tricky. The desktop version looks nice!

    Main thing: responsiveness I see some problems when I resize the screen to a mobile width, it kind of breaks. It works at 376px and lower, but everything in between is a bit wonky. If I resize the window, I get horizontal scrolling. And if I resize within devtools, at 775px for instance, the paragraph text size is too small because of the automatic resizing. On tablet sizes, the background image get pushed to the left, eventually going past the left side of hero__cta and the screen.

    I think these issues are caused by the grid that can't fit the screen.

    Random other things

    • You can add some line-height to the paragraph to gice it some space between lines.
    • There are no error messages for the email input.
    • It would also be nice if the google podcasts logo was centered vertically. Nothing a little flexbox can't fix.

    If you like, you can check out my solution for this challenge as well, it can at least help if you want to make the error messages.

    Don't be discouraged by my comments, you already took a very good step to join the community. Just trying to help!

    Keep on coding! You'll just get better and better.

    Cheers, Joran

View more comments