CSS, CSS Button Creator

Community feedback

• 

  Anton•515

  @antarya

  Posted over 3 years ago

  Hi,

  Great start. It looks really good.

  I have a couple of suggestions, though:

  HTML

  • Wrap your content with the main tag. That way browser will know where to find primary content. Also, it will fix some accessibility issues. Here is a resource with examples related to semantic tags https://learn-the-web.algonquindesign.ca/topics/html-semantics-cheat-sheet/;

  • alt attribute should describe what is on the image. Screen reader users should have an idea of what is on the image based on the alt text. Here are some resources: https://developer.mozilla.org/en-US/docs/Web/API/HTMLImageElement/alt https://jakearchibald.com/2021/great-alt-text/

  • While your class names are pretty good, you might also want to know about another often used format called BEM. Here is a link to the BEM idea http://getbem.com/introduction/.

  CSS

  • I noticed that you are using float to position elements of the plan. I think you can do it much easier using flexbox. Here are some resources: http://flexboxfroggy.com/ https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Flexible_Box_Layout https://css-tricks.com/snippets/css/a-guide-to-flexbox/ https://www.frontendmentor.io/resources under Interactive Tutorials

  • Try not to use fixed width and height, instead use alternatives min-width, max-width, min-height, max-height. That way, you do not restrict your element, and it can adapt to content.

    For example, if you update text inside .summary-plan -> .plan-title and make it bigger, you will notice that at some point, the text will shift down and will be outside of the visual box. You can easily fix it by removing fixed height from .summary-plan and converting all to use flexbox. So, in this case, just removing would be enough.

    For .summary-main instead of fixed width which will ruin your layout on screen less than < 370px you can use max-width: 370px; which will grow or shrink but will never be more than 370px;

    In a real application, you do not control the size of the text; it will be dynamic. So it would be best if you create styles that can adapt to the content.

  • You use the br tag to separate different elements. You will have more control if you use padding/margin on elements. Here is a good resource about margins that will be useful to know: https://www.joshwcomeau.com/css/rules-of-margin-collapse/

  • You may skip .hero-img border-radiuses if you add overflow: hidden; to .summary-main class.

  • You are using align-self, which do nothing in this case. This property is used with flex or grid. https://developer.mozilla.org/en-US/docs/Web/CSS/align-self

  • If you wonder how to remove extra space at the bottom of the image, apply display: block to the image.

  I hope this will be helpful.

  Keep up the good work! Cheers!

  10

  Marked as helpful