Fylo Data storage component nested with CSS Flexbox

Community feedback

• 

  Raymart Pamplona•16,040

  @pikapikamart

  Posted over 3 years ago

  Hey, awesome work on this one. The desktop view looks great, the site is responsive but at mobile state, at around 340px upwards, the site's contents are being hidden by the screen because the 1000gb text part is not responding well to the site's changes.

  Byron already gave a helpful feedback on this one, just going to add some suggestion as well:

  • Since a main tag is needed for every page to distinguish the main-content of the site, replace the section to using main on the .container selector.
  • Remove the height: 100vh on the .container. Avoid using height: 100vh on a large container this makes the element's height capped based on the viewport/screen's height. Instead use min-height: 100vh so that the element will expand if it needs to.
  • Instead of using the max-width on the .container use it on the body tag, this way, even if you zoom out on your screen, the layout will stay consistent or maybe you can just add background-size: 100% on the .container.
  • Remember that a website-logo is one of the meaningful images on a site so use proper alt for it. Use the website's name as the value like alt="fylo".
  • Also, when using img tag, you don't need to add words that relates to "graphic" such as "logo, image" and others, since img is already an image so no need to describe it as one.
  • Those 3 icons below the site-logo are just decorative img tags. Decorative images are just images that doesn't contribute to the overall content of the site. They should be hidden for screen-reader at all times by using alt="" and aria-hidden="true" to the img tag or only aria-hidden="true" if you are using svg instead of img tag.
  • Also, it would be nice to change the span that holds the each of the icon to using a button since it looks like the component is interactive or use div.
  • On a site, always have a single h1. Since there are no visible text that are suitable to be h1, the h1 would be a screen-reader only heading. Meaning it will be hidden visually but present for screen-reader users. On this, the h1 would have like sr-only class and the text-content should describe what is the main-content is all about. The h1 could be placed as the first text inside the main.Have a look at this simple snippet of mine using sr-only h1 comments are already included in there, but if you have any queries just let me know.
  • When wrapping up a text-content, make sure that it is inside a meaningful element like p tag or heading tag and not using like div, span, strong to wrap the text.

  MOBILE STATE

  • Don't use width: 100vw since this will only add a horizontal scrollbar at the bottom, since this value does not account the vertical scrollbar's width.
  • Adjust the .progress-bar so that the site will properly resize and won't create the horizontal scrollbar since the content will be prevent to overflow.

  Aside from those, great job again on this one.

  01

  Marked as helpful
• 

  Byron•2,290

  @byronbyron

  Posted over 3 years ago

  Hi @Mugeez001 Nice work!

  I have a suggestion to make your icons look a bit more uniform, by applying the styles to the .fylo-images span instead of .fylo-images img. Might look something like this:

  .fylo-images span {
      width: 48px;
      height: 48px;
      background-color: hsl(229, 57%, 11%);
      border-radius: 7px;
      display: flex;
      align-items: center;
      justify-content: center;
      margin-right: 1rem;
  }
  
  .fylo-images img {
  }
  

  01