Fylo component using Sass & flexbox - Desktop first

Community feedback

• 

  Dorian Alvarado•210

  @Dorian30Posted over 4 years ago

  Really interesting Jen! I like this solution. I came here, after you submitted your feedback on my solution. I really liked yours! After reading your feedback I thought I could come and learn some things. From my side I have only a few quick notes.

  I would suggest to use more semantic tags for your HTML. Also, for illustrative images like the wave below the page I think it is better to use pseudo elements ::after or ::before and avoid making your HTML more verbose.

  On a side note, I would like to ask you why choosing rem as your first option. I really like how you kept consistency and only used rem, but why choosing that above the others ?

  Also, I saw that in your HTML you wrote font-size: 67% (or some value near that one, I don't remember), but different browsers can have different font sizes. Here I think we are assuming we will have 16px across every browser, or was this intentional ? In that case, what are the reasons for doing that ? Isn't it better to sent a default font size to reset browsers fonts ?

  Looking forward to your answer!

  2

  ---

  

  Jen•1,230

  @En-JenPosted over 4 years ago

  @Dorian30 Thanks for your feedback! You're totally right about using pseudo elements for a cleaner way to add illustrative background images. I just updated mine so that the background image is in the body::after CSS selector. Good call.

  As for your question about using rem for everything (except for the media queries, which I used em for) and setting the base font size to 62.5%, have a look at this article:

  https://engageinteractive.co.uk/blog/em-vs-rem-vs-px#:~:text=A%20typical%20method%20is%20to,rem%20would%20now%20equal%2020px.

  I learned about the technique outlined in that article from the Udemy course "Advanced CSS and Sass: Flexbox, Grid, Animations and More!". It was insanely helpful to me and I highly recommend it if you want to learn a ton about CSS.

  2

  

  Dorian Alvarado•210

  @Dorian30Posted over 4 years ago

  @En-Jen Awesome Jen! Thanks for the quick reply. I will look into it. Actually I saw you did some projects from that course. You caught my interest! I will definitely do so. I have been so caught up with React sometimes.

  If I have any doubts I will probably be writing to you on slack haha! Good talk! I will take refactor my solution to practice this (I have been looking for a good relative based responsive approach)

  One last thing, I noticed you used BEM. I always use maintainable CSS and thought that you might liked it

  https://maintainablecss.com/

  Combined with this

  https://medium.com/wolox/what-margin-padding-taught-me-after-50-projects-6878b53c903f

  2

  

  Jen•1,230

  @En-JenPosted over 4 years ago

  @Dorian30 Just messaged you back on slack!

  1
• 

  Freddy Munive•170

  @fmunivesPosted over 4 years ago

  Hi Jen,

  You had a great job and your styles are very ordered. You're using BEM, that's a good methodologie to work.

  The unique recomendation when you declared transition: all .3s; and later you give the &:hover { border: 1px solid $color-file-icon; }

  For performance it's better to be more specific with the value because all applies for all properties and you just need to apply border property.

  You can check your code in 38 line https://github.com/En-Jen/fm-fylo-data-storage-component/blob/master/sass/components/_files.scss

  2

  ---

  

  Jen•1,230

  @En-JenPosted over 4 years ago

  @fmunives thanks for looking through my code and for the recommendation! Just fixed the transition declaration so that it's more specific. I hadn't ever heard before that using all isn't as good for performance when it's just one property that it needs to be applied to.

  0
• 

  T

  Matt Studdert•13,611

  @mattstuddertPosted over 4 years ago

  Hey Jen, I've only just come across this solution. I don't have much to add beyond the great tips that have already been given. So I'll just say, amazing work! Your solution looks great and scales down really well to mobile.

  You mentioned in the title that you went desktop first. Is that a personal preference? Even if you didn't work mobile-first I'd still recommend using min-width media queries. It can often lead to less CSS code and has the benefit of loading in fewer styles for mobile users, which can be a nice performance gain.

  Keep up the awesome work! 👍

  1

  ---

  

  Jen•1,230

  @En-JenPosted over 4 years ago

  @mattstuddert First off, just want to send you a huge thank you for founding Frontend Mentor 👏 I've had so much fun working on solutions to the challenges you've put out and have learned a ton through doing them. Thanks also for taking the time to check out my solution and giving your feedback!

  Desktop first is my personal preference, but I'm still pretty new to coding and am open to trying a mobile first approach in future projects and using min-width media queries. That's a good point that mobile users will have a performance gain with a mobile-first approach 👌

  1

  

  T

  Matt Studdert•13,611

  @mattstuddertPosted over 4 years ago

  @En-Jen you're welcome! I'm really happy that you've been enjoying the challenges and have been learning a lot while you build the projects 🙂

  Mobile-first can definitely be a tricky way to work at first but after a few projects, you should find that it actually speeds up your workflow.

  Let me know if you ever have any questions about it!

  0