Mobile first countdown timer with BEM and Sass

Community feedback

• 

  Rio Cantre•9,690

  @RioCantrePosted almost 3 years ago

  Hello there! Awesome job with this challenge! Viewing your solution, you did well with the design and functionality. I would suggest the following for you..

  • Adjust the bottom margin of the icons in the .social__icons into 4rem
  • Import the attribution style in the CSS file and remove style tag
  • You can simplify the footer icon with this HTML structure...

  <li class="social__icon">
            <a href="#" class="icon icon--facebook"></a>
   </li>
  
  

  Over all you did well and Keep up the good work!

  Marked as helpful

  0

  ---

  

  Yemisrach•1,430

  @Yemisrach15Posted almost 3 years ago

  @RioCantre Thanks for the feedback Rio🙏🏾. I could simplify the footer as you said but when I tried it, the mask image is invisible. That's why I had to add an empty div.

  1

  

  Rio Cantre•9,690

  @RioCantrePosted almost 3 years ago

  @Yemisrach15 I see. Makes sense. Well you did a good job on this project. Keep it up!

  1

  

  Yemisrach•1,430

  @Yemisrach15Posted almost 3 years ago

  @RioCantre Thank you!

  1

  

  yishak abrham•2,150

  @yishak621Posted almost 2 years ago

  @Yemisrach15 the flip animation should be smooth

  0
• 

  Travolgi 🍕•31,400

  @denieldenPosted almost 3 years ago

  Hi Yemisrach, congrats on completing the challenge!

  I had a look at your solution and I have a few suggestions to improve your code:

  • Try to use Flexbox for center the element in the page: read here ->Flex guide Note: Flexbox aligns to the size of the parent container.
  • To fix the position of the hill image in the background change the value of background-size in contain.
  • Try to adding a little transition on the social hover effect
  • In javascript avoid using innerHTML unless strictly necessary: its use creates a security hazard. Rather use innerText which is the same thing but does not parser html [read here] (https://www.w3schools.com/jsref/prop_node_innertext.asp)

  In the end Instead of using 4 identical functions that perform the same task but with different times, try to use only 1 function by passing it the different parameters.

  Overall you did well :) keep it up and Happy coding!

  Marked as helpful

  0

  ---

  

  Yemisrach•1,430

  @Yemisrach15Posted almost 3 years ago

  @denielden Thank you Deniel. I appreciate🙏🏾. On your first point, is it necessary to use flexbox to center elements? And can you point out where I should use it on my solution?

  1

  

  Travolgi 🍕•31,400

  @denieldenPosted almost 3 years ago

  @Yemisrach15 it's not necessary to use flexbox to center elements but is very simple use it and offer much control.
  In this case you should be use it on wrapper class or to the body

  You can se in my solution here application -> my solution

  Happy coding!

  1

  

  Yemisrach•1,430

  @Yemisrach15Posted almost 3 years ago

  Yes you are right. I'm more familiar with margin: 0 auto to center elements. And nice solution!

  1