Restarting at frontend with Social Links Profile Media

Community feedback

• 

  

  Grace•32,150

  @grace-snowPosted 3 months ago

  Here's some feedback I hope it helps. This looks good overall but there are issues that need improving.

  • it's nit good for performance to load 3 css files like this. Just have it in one.
  • it's better for performance to link fonts in the html head instead of css imports.
  • remove the section from the html. You don't need it and it brings no benefit.
  • this is a component that should probably sit within the main landmark, not be the main landmark. It's fine to use a div for the card inside main.
  • the image and intro really don't need wrapping in extra divs. It's not a problem, but You're just creating more work for yourself doing that.
  • the name is a heading, but the location is not. That's just a paragraph. Think about it, is "London" a heading for all the content underneath it? No. So it's not a heading. I wrote a post all about headings on FEDmentor.dev if you want to look it up.
  • personally I would have the quote in a blockquote element and place the list of links in a list not div.
  • the css looks fine overall, just a bit overcomplicated. You're using more selectors than necessary I think but some of that comes down to the overly complex html as already said. Once that is simplified the css can be simplified.

  1

  ---

  

  Leonardo•40

  @LeonCelestinoPosted 3 months ago

  @grace-snow I'm really appreciated for the feedbacks. I'll apply the changes.

  0

  

  Leonardo•40

  @LeonCelestinoPosted 3 months ago

  @grace-snow Hello, Grace. How are you doing?

  I tried to apply the suggestions. Would you be free to check it? Thank you very much

  0

  

  

  Grace•32,150

  @grace-snowPosted 3 months ago

  @LeonCelestino Looks good now, well done

  1