Fylo dark theme landing page | SASS | 7-1 Architecture | Mobile First

Community feedback

• 

  P

  Dave•5,295

  @dwhensonPosted about 3 years ago

  Hey @krisp-dev I just posted a long comment here, but it got lost when I tried to post it so I'm sorry if this is a bit short! Overall great job 🙌 here's some points to think about:

  1. You might want to think about stopping the page from spreading too wide a very large screens. There are many ways to do this but I set a grid on the body element, with three columns, as using a class selector as follows:

  .center-content {
  	display: grid;
  	grid-template-columns: minmax(1rem, 1fr) minmax(375px, 1440px)minmax(1rem, 1fr);
  }
  
  .center-content > *  {
  	grid-column: 2;
  }
  

  The 1440px is the max-width you want the main content to be, and the 1rem values is the smallest spacing you want either side of the main content on small screens (I sometimes put this to 0 and use a container to add padding to each section).

  The second part positions all direct children of the body in this nicely sized middle column. In my case, mostly, my header, main, and footer the middle column, and stops them going wider than 1440px. It’s also pretty easy to ‘break’ elements out of this constraint if you need to.

  Other people use container classes to do the same thing. Either way it's a good idea to find an approach that works for you as you'll need this for a lot of FEM challenges.

  2. On form validation, at the moment the in built validation runs on focus so I get an error before I even type anything. I think the inbuilt validation is a good fallback for some progressively enhanced validation using JS (if you are comfortable doing that...)

  For example, I would typically do the following:

  • On page load, add a listener to the form to i) prevent it being submitted by default, ii) add a 'novalidate' attribute to the form (as we are doing this with JS now, but if JS fails the inbuilt validation would still work), and then run a function to check the email validity.

  • Not sure how comfortable you are with JS, but then if the email validity fails, I then add another listener on the ‘change event to check the input every time someone types in the input, this then checks when the email is valid immediately and removes the error styles.

  Sorry if that sounds hectic, you can see an example in my code here:https://github.com/dwhenson/frontend-mentor/blob/master/28.%20fylo-landing-page/js/main.js

  Having just looked at this again, I would change the initial event listener to listen for the submit event, rather than click, and I would change the keydown event to change - as this will also validate if people paste a value in rather than type.

  Overall though lovely job!

  Cheers 👋

  Dave

  Marked as helpful

  2

  ---

  

  Kris Pietrzak•340

  @krisp-devPosted about 3 years ago

  @dwhenson Thanks for the constructive and insightful feedback Dave!

  I have written it down in my notes to make sure to check how the design looks like on much larger screens. My screen is only 1920px wide, and it still looked alright but I will have to get used more to the dev tools as many people nowadays may have the 4k 32" curved monitor extravaganza :)!

  I usually make sure to set max-width to 1440px while doing these challenges when doing Desktop to Mobile (my preferred way), but I am practicing the Mobile first approach as it seems to be the industry standard and totally forgot about it.

  As for the email validation, I will come back to this challenge and fix it once I get more comfortable with JS, as I am still in the beginning stages of learning it.

  Thanks again! Much appreciated!

  0

  

  Kris Pietrzak•340

  @krisp-devPosted about 3 years ago

  @dwhenson Just to follow up, after playing around with it I have ended up using your 'trick' with the CSS Grid (.center-content) and it works great.

  Escaped some of the content (Nav, Header & Footer) using grid-column: 1 / -1 to make sure that the background colors & header image is spanning across all 3 columns and the main content stays inside grid-column: 2 - hope I am making sense here :)

  Also had to add a new media query for screens larger than 1920px and adjusted some paddings. Overall it looks way more solid and robust on larger screens now.

  1

  

  P

  Dave•5,295

  @dwhensonPosted about 3 years ago

  @krisp-dev great glad it helped! Yes, that's exactly how to get things to span full width when I need too.

  One issue you might run into is when you need to escape sections that are not at the top level within the grid, for example a section inside the main element. Because they are not direct children they aren't grid items so the above technique won't work.

  If and when you come to that problem, just remember this link: https://piccalil.li/tutorial/creating-a-full-bleed-css-utility/ applying this class to the section you want to go full width should sort the problem mostly....

  But then your content will probably flow the full width of the screen. So to sort it out I simple add the .center-column class onto the same element to constrain the width again...

  This probably sounds a bit hectic, and I wouldn't worry about it until you need to then, perhaps check here and see if this makes sense - or give me a shout here or on the Slack and I'll be happy to help.

  Cheers

  1