Single Price Grid Component Master - Newbie Challenge 03

Community feedback

• 

  Szymon Rojek•4,540

  @SzymonRojekPosted almost 4 years ago

  Hi Georges,

  I have checked your HTML structure by inspecting a project, a few tips from me:

  • instead of an article maybe it will be better to use a section, what do you think?

  • I know this is only a component but also you can see it as a single page component so you could do h1 with to spans inside of it named: main-heading(Join our community) and sub-heading(30-day, hassle-free money back guarantee) but also a sentence Join our community can become h1, underneath will h1, and last - paragraph. Also, you can leave it like this but add the h1 tag with the class sr-only hidden for Screen Readers;

  • using ID as a class it is a bad idea (they can be used with JS later on);

  • dollar sing and per month you can put inside of the paragraph with two spans;

  • in the third box we have got a good occasion to use unordered list and list item (br tag is unnecessary here);

  • what do you think about link instead of button? Also, submit => we are not submitting here at the moment we just want to go to a different part of a page;

  • removing outline from a button it is not a good practice.

  Design

  • on mobiles: you can add padding to the text under the light green color text "30...";
  • add overflow hidden to the body (eliminate the scroll).

  That's from me. Ps. Don't forget to upvote any comments on here that you find helpful.

  Greetings :D

  1

  ---

  

  Georges Creates•160

  @georgescreatesPosted almost 4 years ago

  @SzymonRojek Hey! Thank you so much for your tips. I will take them in consideration.

  1

  

  Szymon Rojek•4,540

  @SzymonRojekPosted almost 4 years ago

  @georgescreates

  Great. I have mentioned something about a link and button so this article from the CSS-tricks: A Complete Guide to Links and Buttons will be very useful. Read it carefully.

  That's from me :D

  1

  

  Georges Creates•160

  @georgescreatesPosted almost 4 years ago

  @SzymonRojek How are you doing ? I just updated the code of this challenge, following the tips you gave me. Thankfully, they helped me.

  • when I was starting this project, I had in mind that it was a part of a page. So, I thought I could not use h1 anymore, like, obviously, it was yet used. But yeah, your approach make me think otherwise.
  • I did not know for the usage of id and class. Also, for buttons which removing the outline is not a good practice. I read the article from css-trick you sent. I have to go deeply with practices.

  The only thing that I could not modify was the hidden overflow on the body. Does this mean, on mobile version, users won't actually need to scroll for this section ? Like, it can be fit in the whole viewport height without having to scroll ?

  Thanks for your considerations.

  1

  

  Szymon Rojek•4,540

  @SzymonRojekPosted almost 4 years ago

  @georgescreates

  I am glad that you change some thins in your project, but I have a few tips more:

  • I'd recommend reading more about BEM - naming convention, generally your classes are not very readable;
  • ID's => just try to not use them in the next project;
  • I don't remember but I think we don't need two span inside of a paragraph, maybe only for "per month";
  • also check your project by the inspector in your browser (especially on 980px heading => these to sentences are inline);
  • now your project looks fine without the scroll (Sorry, I didn’t write it clearly. I was thinking to add this when you have got full size of your project, also it is good to use 100vh);
  • do you have to use in the body width: 100%; height: auto ? I think this is unnecessary, also in another tags (when I am using flexbox and grid I am trying to not using any height and width explicitly - if I have to I am using min-height, min-width => before I used to do it a lot);
  • display block for h1 is not needed;
  • when you have got border-radius: 0px 0px 0px 7px => you can simply type just border-radius-left unless you want to reset top, right, bottom;
  • 2 zeros:

  #month-part p:nth-of-type(2)
  {
      margin: 00px 0px;
  }
  

  • the bg-color in third section is very bright - it is hard to read text. Another solution for it, you can give the same bg-color to the 2 and 3 box, then in the third box you can do something with pseudo-element ::after (play with opacity);
  • put max-width to the container because at the moment is growing with the view port width (check it out);

  Important: I'd suggest learning more about flex and grind in the height and width context (I didn't use height explicitly in my project);

  Don't forget to upvote any comments on here that you find helpful.

  Greetings :D

  1