profile-card-component using React, Flexbox, and SASS.

Community feedback

• 

  Raymart Pamplona•16,100

  @pikapikamartPosted about 3 years ago

  Hey, really nice work on this one. The overall layout looks great.

  Here are some suggestions for the site:

  • For this one or just in general, when you building a site, always have a single main tag to wrap the main-content of that page. For this one, the .main should be using a main tag instead of div so that the site will contain a main-landmark.
  • Those 2 blobs images could be used as background value. This way, you won't have to create those 2 div.
  • For the div that wraps the .attribution, use footer tag on it so that it will be nested inside in another landmark. So on this page, you would have a markup that looks something like this:

  <main />
  <footer />
  

  • Remove the position: absolute on the .main since it is not needed, this just hides the .attribution as well. Remove the position: relative from the div that wraps the .attribution. For this, you don't really need them. So you will have to remove the stylings:

  position
  left
  bottom
  

  Since the .App is using a flexbox already to center both items.

  • Also on the .App, remove the height: 100vh. Avoid using height: 100vh on a large container as 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.
  • For this one, since the layout is showing a person's information and statistics, it makes sense to use the person's name as the person's img alt value.
  • You could use for now the h1 to wrap the person's name since a single h1 is needed for a site. If you feel like h1 is not suited on this, use h2 on the name and have a search about sr-only h1.
  • 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 to wrap the text.
  • For the .stats selector, if you look at the content of that part, those are "list" of information about the user, it would make sense to use a ul or dl tag in that one.

  Aside from those, great job again on this one.

  Marked as helpful

  0