Social Links Profile

Community feedback

• 

  T

  Grace•31,270

  @grace-snowPosted 18 days ago

  So there are some important principles to learn from these early challenges. I'm afraid a lot of this will need rewriting. Work through the feedback carefully and take time to understand. You will probably need to go back through earlier challenges and apply the same learnings.

  • All content should be contained within landmarks.Everything in this design belongs in a main landmark. (What you have as a div with the class container).
  • The image, name heading and paragraph are not links! Where do you think those would even go to? I've no idea what the intention was there, but these are definitely static page content and not links.
  • The image is decorative in my opinion so should have an empty alt attribute. If you think the image is providing valueable/meaningful content, then the alt description would need to properly describe the image.
  • The top half of the card doesn't need wrapping in a div. Keep the HTML as simple as possible. All extra wrappers do is make the styling harder for no benefit.
  • I'd expect the list of links to be in an actual list.
  • You don't need paragraphs inside the links.
  • Always use a full modern CSS reset at the start of the styles in every project. Andy Bell or Josh Comeau both have good ones. Look them up and use one every time.
  • Content is currently being badly cut off at some screen sizes (like smaller devices, especially in landscape view). This is because you have limited the container height to 100vh. Never limit the height of any elements that contain text anywhere within them. Instead, use min-height so the browser is allowed to extend that element when it needs to.
  • I think you're getting confused about when to use padding and margin. This article should help: https://fedmentor.dev/posts/padding-margin/ . The main and card should both have padding. Everything within that card should only need vertical margins. The links will need padding on all sides. The image can have margin auto to center it, not an explicit margin in px.
  • Nothing except the image should have a width or height in this. This is a very important principle to understand. It's the browser's job to decide what height things need to be based off the content inside them and any internal spacings.
  • Instead of width on the card, use max-width in rem (it can optionally be width 100% as well). This lets it shrink narrower when it needs to and using rem means the layout will scale for all users, even those with different font-size settings in their browser or device.
  • There is no need for a media query in this challenge.
  • Remember, it is the links that need styling, including hover. Not their wrapping elements. Keep it simple.
  • You can use flex properties to center the text in the links.
  • You can set text-align center on the whole card component and it will be inherited by the child elements.
  • Font size should never be in px. Use rem. https://fedmentor.dev/posts/font-size-px/

  0