Flexbox

Community feedback

• 

  Edgar Burtnieks•465

  @edburtnieks-privatePosted over 4 years ago

  Hey Frederik! What exactly would you want to improve and focus on?

  0

  ---

  

  Frederik Kempe•65

  @MeFredjPosted over 4 years ago

  @edburtnieks I would like to make sure I used the right mobile responsiveness code. I do realise there are many options, I'm just wondering if I'm going the right way instead of overcomplicating things.

  Thanks in advance!

  0

  

  Edgar Burtnieks•465

  @edburtnieks-privatePosted over 4 years ago

  @MeFredj Okay, so I will try to give some feedback on responsive design:)

  In general everything seems okay and quite good. I noticed couple small things.

  First, I don't know why you have set height to body element of 900px. It cuts the background image with straight line and doesn't look that good. I think in most case, especially with background image with visible seam, you should have the body element take up the height of viewport.

  Try to use relative units such as em's or rem's for font-size. I see you have set it as 2.5rem for h1 on desktop. So for mobile you can set it to rem's too. Just makes life a lot easier, predictable and maintainable.

  I see that you have not used mobile first approach. Meaning, to define all the default styles for mobile screen first and change necessary properties in media queries for desktop using (min-width: ...). It is not necessary, but in my experience it is a lot easier to do this approach and for me it is even easier to read and know that all the styles are for mobile, and all changes will be inside media query for tablet, desktop screens. It doesn't not necessary mean you have to start with mobile screen. In fact I most often start with desktop and move downwards, keeping in mind to use this approach. One advantage of using this approach instead might be that you wouldn't need to specific display: flex; for mobile and only add it to tablet and larger screen sizes, as right now you have block elements witch will stack on each other by default. Also I don't think flex-wrap: wrap; on .flex-container is not necessary.

  There seems to be some issue at around 786px screen width where text becomes centered, is right against the image. I believe it should be below the image too. The tips I mentioned above should fix this issue ;)

  One final small detail I could give is to set box-sizing: border-box; to all elements using * selector. I noticed that you specified exact width and height for .circle inside footer, however the actual dimensions are 43pxx43px because you are adding border which contributes to total width at the end. Setting box-sizing: border-box; fixes that.

  I this helped, let me know if you have any questions :) Keep it up!

  1

  

  Frederik Kempe•65

  @MeFredjPosted over 4 years ago

  @edburtnieks Thank you so much for your valuable feedback. Especially about the display: flex property. This is something that would make things a lot easier. Also the border-box value is something I should not forget. And yeah, I should have used rem's instead of px's.

  Once again, thanks for your time and effort. It is really appreciated!

  0