My solution

Community feedback

• 

  Szymon Rojek•4,540

  @SzymonRojekPosted almost 4 years ago

  Hi Yuniac,

  Good job! :D

  I have checked your project by the inspector, a few tips for me:

  • remove this <div class="container"> and create the main tag with the class;
  • generally you have used so many unnecessary nested divs (try to change it);
  • inside of the main tag I could use the section tag, and then three divs for these three boxes;
  • about this code below:

  div class="wrapper sub-price"><span class="dollar-sign">$29</span><span id="permonth">per month</span></div>
  

  Instead of this div you I'd recommend using the p tag with two spans (ID's are unnecessary here => it is good to use them with JS); Below will be the *p tag with this text: Full access for less than $1 a day

  • what do you think about the link instead f a button? (this on click doesn't have to be here);
  • third box: Why us => you can use the h2 tag;

  Design:

  • check your project on different devices by the inspector in your browser, there is lots of to do: play with the size of the text, margin (especially the top, right, left), padding, the footer;
  • text in the first box: these two paragraphs under the green color text should be on mobile treat as a one text but later you can get this solution => separate sentences one under each other;
  • per month: color of this text is not very well (check the design file);
  • this is single page component so you can add overflow hidden to the body (eliminate the scroll);
  • the bg-color of the third box: I gave them the same color for the second and third box but additionally I have added pseudo-element after to the third box with the gray color => I called it "overlay";

  That's from me.

  Greetings :D

  1

  ---

  

  Yuniac•65

  @YuniacPosted almost 4 years ago

  @SzymonRojek Thank you for your feedback. Yes, I have this habit of using many nested devs, I need to change it, as for the colors yeah I mentioned that I didn't get them right, will give the design guide file another read. As for the responsiveness of it, it could use some work that is true, anyway, thanks again!

  1

  

  Szymon Rojek•4,540

  @SzymonRojekPosted almost 4 years ago

  @Yuniac

  Great! don't worry, it will take some time to achieve - we have to learn lots of things. Generally yes, using nested divs it is not a good practice - that's why we have got flexbox, which is good for the elements inside of the parent. Have you ever try to use pseudo-elements?

  Cheers :D

  1

  

  Yuniac•65

  @YuniacPosted almost 4 years ago

  @SzymonRojek Only the links related ones (hover, visited, etc) and nth-of and first/last-child, what do you think I could have done differently with them here? or why are you mentioning them?

  0

  

  Szymon Rojek•4,540

  @SzymonRojekPosted almost 4 years ago

  @Yuniac

  Generally when you are creating links or buttons you have to ask yourself: are you giving a user a way to go to another page or a different part of the same page? In this project I prefer a link (a href="/somewhere">link</a).

  I can recommend this article from the CSS-tricks: A Complete Guide to Links and Buttons. It will be very useful.

  :D

  1

  

  Szymon Rojek•4,540

  @SzymonRojekPosted almost 4 years ago

  @Yuniac

  Also, this article => When To Use The Button Element.

  Read two of them a few times.

  1

  

  Yuniac•65

  @YuniacPosted almost 4 years ago

  @SzymonRojek Will check them out both, thank you for your input and time.

  0