Fylo dark theme landing page || Responsive || Mobile first

Community feedback

• 

  P

  Dave•5,295

  @dwhenson

  Posted almost 3 years ago

  Hey @Drougnov lovely job here! The page looks great!! 🥳. Here's some things to think about:

  • Personally, I never worry too much about pixel perfection. I go for "pixel pretty close" here's a great article about this: https://www.joshwcomeau.com/css/pixel-perfection/

  • The quote should be on top of the review section? It is on my screen - can you clarify the issue there? Sorry if I'm missing something obvious!

  • And again for the animation - which 'bottom part' would you like to animate? You can do most things without a library, but in some cases it might be a lot of work!

  Lastly, 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, 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 value is the smallest spacing you want on 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 are the middle column and stops them from 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. This article has a good run down of alternative approaches https://css-tricks.com/the-inside-problem/ You will note I am actually using the approach the author doesn't like!

  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.

  Cheers

  Dave

  11

  Marked as helpful
• 

  Vanza Setia•27,715

  @vanzasetia

  Posted almost 3 years ago

  Hi, @Drougnov! 👋

  Regarding your questions.

  • For the background image (bg-curvy), I almost did the same thing as you did. But, I set the background-size: 100% and in my opinion it made looking similar to the design. As a side note, I would not recommend focusing to make your solution as "pixel-perfect". But rather, focus on the code quality more especially you just get started. There's a blog post by Josh Comeau about chasing "pixel-perfect" dream.
  • For the z-index, I can't figure out how to make the pseudo-element to go behind the relative element. I tried removing the z-index from the .review_section .first-review and the result is the quote icon disappear. I tried playing around with it but still no result. So, my recommendation is to make the quote icon as a background image of the .review_section.
  • I am not sure with your question. Do you mean a scrolling effect (like this)? Or are you talking about the form validation?

  About the z-index, on my solution, I used img element to show the quote icon and I managed to put the quote icon behind the card by doing the following.

  .testimonials__card {
      position: relative;
  }
  
  .testimonials__quotes {
      position: absolute;
      top: -2.5rem;
      left: 0;
      z-index: -1;
  }
  

  It's straightforward and the important thing, it is working! 👍

  So, maybe you can inspect my solution or try to use img element or may be make it as a background-image of the .review_section. (I can't explain about the issue 😓)

  Now, for the feedback.

  • The alternative text of the logo should be the name of the company. It is an information that screenreader users may use to know the site/project name.
  • For the aria-label, I recommend making sure that it is a link but at the same time telling about the image, like "Home | Fylo". If you choose to use aria-label as the text content of the link then leave the alt empty. The same goes for the opposite, if you choose to use alternative text of the image for the text content of the link then remove the aria-label.
  • If the link is already having the text content then there's no need to use aria-label.
  • Wrap the nav element with header. The first section that contains the h1 should be the first section of the main element. header is used to wrap navigation element while main landmark is used to wrap the main content of the page.
  • The illustrations are decorative images. So, leave the alt="" empty.
  • I would expect the "Get Started" button as a link that navigates the users to sign-up page or something like that. So, unless you expect different thing to happen after the user clicks the button then it would more likely to be a link.
  • Every time you use the button element, you need to always specify the type of the button. It will make sure that the button behaves properly across multiple browsers.
  • aria-label is used to add text content for link element that doesn't have text content. A good example would be the social media icon link. Also, it works the same as the visible text content, is should tell what is the link for. So, if it is a facebook icon then the aria-label="Facebook" would be good enough to tell the screenreader users about the link.

  That's it! Hope this helps.

  12

  Marked as helpful