Huddle landing page with single introductory section solution

Community feedback

• 

  T

  Grace•31,270

  @grace-snowPosted about 2 months ago

  There are some issues in this I notice. I hope this helps.

  • the logo is not a nav element. The nav landmark is for navigation not logos. Ideally the logo should be in a header outside of the main landmark.
  • the logo alt must say the same as the logo. That is essential.
  • the main image alt needs to describe the image if you consider it to be meaningful content. Remember it's content not code. if you don't think it's meaningful content the alt can be blank.
  • this is hard to read and spot bugs because code isn't indented consistently. Your code editor can even do this formatting automatically for you with prettier.
  • register should be a link not a button because it would navigate somewhere on click. (Submit buttons in particular are only for forms).
  • the social icons are not just icons! This is a really important foundational skill – learning to translate a design into appropriate html. Think it through. Wouldn't you expect to be able to click on these on a real site? The design even includes a hover style so you know they should be interactive. That means you must wrap them in links and make sure that they have an accessible name. If using font awesome icon fonts that means you'll need to add sr-only text for the link accessible names OR add aria-label on each one.
  • don't forget to update your attribution link so it points to your frontend mentor profile page or github profile page.
  • it's better for performance to link fonts in the html head instead of css imports.
  • get into the habit of including a full modern css reset at the start of the styles in every project. What you have does not constitute a full reset. It's strange actually as you seem to have made up your own reset but it's not at the top and it's mixed up with site specific styles... Andy Bell or Josh Comeau both have good ones you can look up and use.
  • place the background on the body not html. You pretty much never want to style the html element, just leave it alone.
  • there is no reason to place percentage widths on elements like headings and paragraphs. You could set max width on them in rem or ch units but not percentages that's a strange choice that removes your control.
  • I think what you're actually missing in this layout is a max-width for the whole content/layout to stop it getting too wide, maybe that's why you've added these percentages I'm not sure... The max.width should be in rem. In this project you'd add that on the main landmark but it's also usual to need to set that max width on other layout elements when building other pages, it won't always necessarily go on landmarks.
  • it's really strange to set all font sizes to be the same size!
  • the logo size should be a fixed value. It definitely shouldn't have its width in viewport units which removes all control. Give it a width and height or one of those and.aspect ratio.
  • overall this whole layout would be simpler with css grid I think. Just a suggestion not necessary.
  • the register link shouldn't have a width either. All it needs is padding. That's where the width comes from naturally (content plus padding) same as with it's height.
  • it doesn't matter so much here as you need to change it to a link from button anyway. But so you are aware for future, don't remove borders altogether from buttons. Instead give them a transparent border or make the border colour the same as the background colour. This is so that the buttons display correctly for.users who have high contrast mode turned on in their browser or device.
  • it's not wrong using rem for line height, that's fine, it's just a bit longer than usual. Usually line height is unitless like 1.5. It's calculated by dividing px line height by font size. Eg in a design file a line height of 24px and font size of 16px equals a line height in code of 1.5.
  • media queries should be defined in rem or em not px so that layouts adjust at a good time for all users including those who have a different text size setting in their browser or device.

  Reading these styles overall it seems like they are well.considered and you've tried to structure them well. It's great you've included comments to split sections out and try to help readability. But I can't help but feel they are more complicated than they need to be... I just wouldnt expect such long styles for this project... or maybe it's all the media queries being scattered through it that makes me feel that way. I read loads of code all the time and just get an instinct that this could be shorter/simpler overall.

  Marked as helpful

  1

  ---

  

  Cas•220

  @iguanasplitPosted about 2 months ago

  @grace-snow Hey! I'm a few days late but i wanted to really thank you for taking the time to give me so many pointers and giving advice on so many details. i will redo the challenge and try to see them fixed. and thank you!

  0

  

  T

  Grace•31,270

  @grace-snowPosted about 2 months ago

  @iguanasplit no need to "re-do". Just make updates and push that new code to the same repo which will update this solution

  Marked as helpful

  0
• 

  T

  Grace•31,270

  @grace-snowPosted about 2 months ago

  You should never set height anywhere. The only exceptions are sometimes icons, images or decorative items. You never want to limit the height of any elements that contain text, including the body or other landmarks.

  Most of the time you can just ignore height. Instead, let the browser do it's job and decide what height is needed based on the content inside.

  To center all the content on the page you can use min-height 100svh on the body. Note the difference with min-height (not height).

  Marked as helpful

  1
• 

  P

  Hazel Dotson•300

  @Hazel-BlackPosted about 2 months ago

  Hey Cas! So Yes there are lots of improvements to be made but I think its great that you are getting back into this and trying. I took a long break from coding as well but it feels amazing to be back😊. Anyways, I said all that to say I'd love to work with you. I'm not an expert but I can help you ease back into coding and we can learn together. I'm hazelbdev on discord. feel free to reach out.

  1

  ---

  

  Cas•220

  @iguanasplitPosted about 2 months ago

  @Hazel-Black hey! Hazel, I'm a few days late but id like to take you offer. i too think its great to be back and I'm glad you feel similarly. id love the help and thanks for being so welcoming i go as iguanasplit on discord! see you there

  0