Social Links Profile using Tailwind CSS

Community feedback

• 

  T

  Grace•31,270

  @grace-snowPosted about 2 months ago

  Hi, this looks good but has unnecessary media queries and the html has some major problems.

  Take a moment to try and use this with your keyboard. Even with a mouse try and click the social links. That should show just some of the issues but there are some more I'll try to explain.

  • there is no reason or benefit to using a header element inside the main landmark. It's no longer a landmark when you do that, header becomes just the same as a div. You don't even need an extra wrapping element around that content. Keep it simple. This is only a card.
  • the alt "profile picture" brings no benefit as a description. The alt either needs to describe what the image looks like or be left empty so the image is treated as decorative.
  • the location London doesn't make any sense as a heading for all the content underneath it. That line is really just a paragraph.
  • as this is a social links card it's essential to use links (anchor elements). Without that the whole component is non functional. Alarm bells should be ringing in your head whenever you find yourself adding hover styles to a non-interactive element. Always pause and think through the html carefully.
  • I recommend you use a flex column approach to centering the component on the screen and not the grid place content center approach. The flex column way is just more robust. As you have it currently if someone enlarges their text size on a mobile screen some content can become cut off and unreachable on the top and left. That wouldnt happen with a flex column.
  • there's no need for any media queries in this. The component doesn't need a width. Instead just give it a max width in rem. Then there will be no need for media queries.
  • make sure the component can't hit screen edges. Add a little padding to the body or main so that can't happen.
  • generally css imports are poor for performance. I've not seen npm packages imported like that before though so am unsure of the implications. I would expect you to have used npm to import the font locally then link it to the local location in the html head. But maybe your method is specific to the package and there's something I don't know about it.

  Marked as helpful

  1

  ---

  

  P

  Jaylen Baxter•300

  @JYLNPosted about 2 months ago

  @grace-snow Thank you for your feedback! I appreciate the reminder to think about the concepts of HTML and CSS. I got too focused on replicating the design provided than what this component should actually look like and perform like in practice.

  As far as the import is concerned, it is imported that way due to the dependency resolution that Parcel performs at build time. I was getting a build error while coding this until I gave it the npm: in front of the import string. Just judging from the code in the dist folder from the build process (at least by default as I don't have any custom Parcel configurations applied), the imported font is placed in the dist folder and then @font-face declarations are placed at the top of the built CSS file. So there is no import in the compiled CSS from the build process. I'm not sure what performance implications are involved in this process of having essentially local fonts as font-face versus the link in the HTML, but that's something I'm going to research moving forward. In the meantime, I did change from using the fontsource font package to linking the font from Google Fonts.

  I made changes based on the other feedback you provided, and will continue my further solutions with this knowledge as well.

  Thank you again!

  0

  

  T

  Grace•31,270

  @grace-snowPosted about 2 months ago

  @JYLN if it's just applying font face directly in the css and not using import at all then that's great, there won't be a performance hit

  0

  

  T

  Grace•31,270

  @grace-snowPosted about 2 months ago

  @JYLN if it's just applying font face directly in the css and not using import at all then that's great, there won't be a performance hit

  1

  

  T

  Grace•31,270

  @grace-snowPosted about 2 months ago

  @JYLN why have you added a nav element?! I don't think you understand what a nav is. It's definitely not this. That would only be used for navigation within this site, not for going to other sites. It would make sense for them to be in a list but not in a nav.

  1