All comments

• 

  tired-herb•120

  @tired-herbSubmitted about 1 year ago

  Hello, everyone! Could you help me with my icons, please? I don't really like them.

  

  Huddle_landing_page-with_single_introductory_section

  1

  60
  

  Ed-dyb Abdelilah•1,560

  @eddybproPosted about 1 year ago

  Hi, @tired-herb👏

  Congratulations on finishing this challenge, I hope it was fun🤣

  For the icons issue here is a way to fix it:

  • HTML code:

  <address>
      <div class="icon-box">
       <i  class="your-icon"></i>
     </div>
     <div class="icon-box">
       <i  class="your-icon"></i>
     </div>
     <div class="icon-box">
       <i  class="your-icon"></i>
     </div>
  </address>
  

  • CSS code:

  address{
    display: flex;
    align-items: center;
    justify-contents: center;
    gap: 1rem;
  }
  
  .icon-box{
    width:2rem;
    height:2rem;
    border: 1px solid white;
    border-radius:50%;
    display: grid;
     place-items: center;
  }
  

  That's it Happy Coding

  0
• 

  Moritz Rose•90

  @moritzroseSubmitted over 1 year ago

  Please let me know anything, you think that can be optimized to make it more responsive or better in general, still very new to this!

  

  3-column-preview-card-component

  2

  60
  

  Ed-dyb Abdelilah•1,560

  @eddybproPosted over 1 year ago

  Hi, moritzrose

  Congratulation on finishing this challenge, I hope it was fun.

  For the code, You did a nice job but still need some improvements mostly in CSS:

  • CSS code:

  @media only screen and (max-width: 375px){
     .container {
          /*remove ''margin-left'' and ''margin right'' properties*/
  }
  .container div{
      /*change the "padding" values to (25px=1.56rem)*/
      /*add "padding-bottom" property*/
       padding-bottom:1.56rem;
       /*remove the "height" and "position" properties*/
  }
    button{
      /*remove the "position" property*/
  }
  }
  
  @media only screen and (min-width: 376px) and (max-width: 1440px){
      .container {
          /* remove the "height" property */
          /* remove this : align-content: center; */
          /*add this */
          justify-content: center;
      }
     .container div{
     /*update this*/
     padding:1.56rem;
  }
    button{
     /*remove "position" property*/
  }
  }
  

  Tips

  • Remember in most cases you don't have to use the height property in containers like div or main ..., instead you can use min-height, Which makes your site displays well in small screen sizes.

  • Use relative units rem em instead of absolute units px.

  • Make use of HTML semantic tags: header, main, footer.

  • I recommend you start using classes and, or pseudo-elements: example:

  • • In the button you can use a class like that: <button class="btn"><button>
  • • Here is how to use pseudo-element:

  .container > div{}
  

  Using classes and pseudo-elements makes your styles display faster.

  That's it HAPPY CODING

  1

  This comment has received 3 repliesShow
• 

  Jouter•90

  @JouteeSubmitted over 1 year ago

  I'm not sure about the media section and if it should be used or if it could be done without it. I'm also not sure if i used right responsive units. I tried to use more semantic elements than just div, but I don't really know where to use which. I appreciate any advice.

  

  input forms with js validation, responsive design

  1

  20
  

  Ed-dyb Abdelilah•1,560

  @eddybproPosted over 1 year ago

  Hi, Joutee

  Congratulation on finishing this challenge, I hope it was fun.

  For the code here are some suggestions that might help improve it:

  • CSS code:

  body{
       /*add this line*/
      overflow: hidden;
  }
  

  • JS code:

  //You did not check for the leap years, eg: 2000;
  // here is a way to deal with it:
  if(year % 400 == 0 || (year % 100 != 0 && year % 4 == 0)){
         //leap year
         //Update the value of the second month
          daysOfMonths[1] = 29;
      }else{
          daysOfMonths[1] = 28;
      }
  //You can also add the prefix 0 in front of values in the inputs like this:
  inputEle.addEventListener('change', ()=>{
          if(inputEle.value.length==1){
              let val = inputEle.value;
              inputEle.value = '0' + val;
          }
      })
  

  Tips

  For the CSS UNITS, The em unit is relative to the font size of its parent element, while the rem unit is only relative to the root font size of the HTML document. The “r” in rem stands for “root”.

  That's it HAPPY CODING

  Marked as helpful

  0

  This comment has received 1 replyShow
• 

  Carlos Tabangay•80

  @RedDotz20Submitted over 1 year ago

  Any suggestions how can I improve my css especially with the min and max width of both result & summary component; I do think there is some better way to do it and make it more responsive.

  

  Result-Summary component using vanilla HTML/CSS

  1

  20
  

  Ed-dyb Abdelilah•1,560

  @eddybproPosted over 1 year ago

  Hi, RedDotz20

  Congratulation on finishing this challenge, I hope it was fun.

  For the code here are some suggestions that might improve it:

  • CSS code:

  #app, .result-container, .stats {
  /*add this line*/
  border-radius: 1rem;
  }
  .result-container {
  /*update the width and min-width properties*/
  min-width:22rem;
  width: 50%;
  }
  
  .summary-container {
  /*remove max-width property*/
  /*update the width property*/
  width: 50%;
  /*add min-width property*/
  min-width: 22rem;
  }
  

  that's it HAPPY CODING

  Marked as helpful

  0
• 

  RohyllerP•70

  @RohyllerPSubmitted over 1 year ago

  

  Responsive landing page JUNIOR SASS HTML

  1

  10
  

  Ed-dyb Abdelilah•1,560

  @eddybproPosted over 1 year ago

  Hi, RohyllerP

  Congratulation on finishing this challenge, I hope it was fun.

  Here are some suggestions that might improve the code:

  • CSS code:

  .p-all{
  margin: 0;
  }
  
  .main{
  margin:0;
  }
  

  Tips

  • Syntactically the main element is a semantic element and should be the container like that:

  <main>
  <!- your code here->
  </main>
  

  In general, the HTML file structure looks like that:

  <!DOCTYPE html>
  <html lang="en">
  <head>
  </head>
  <body>
  <header></header>
  <main></main>
  <footer></footer>
  </body>
  </html>
  

  • It's not necessary to add units to the values that equal 0.

  e-g: margin: 0;

  • Remember to use relative units rem em instead of absolute units px.

  ALL DONE YOU DID A NICE JOB KEEP GOING

  0
• 

  Ornelle•70

  @BlackysynchSubmitted over 1 year ago

  Hi all, this is my solution to the order summary component challenge.

  I managed to use media queries to specify display for small and large screens.

  Any and all feedbacks is welcomed...

  

  Order summary component with simple HTML and CSS

  1

  60
  

  Ed-dyb Abdelilah•1,560

  @eddybproPosted over 1 year ago

  Hi, Blackysynch

  Congratulation on finishing this challenge, I hope it was fun.

  Here are some suggestions that might improve the code:

  • CSS code:

  @media only screen and (min-width: 376px)
  .card {
  background-color: white;
  /* remove this line: height: 80%; */
  /*  remove this line: min-width: 30%; */
  /*update this*/
  max-width: 25rem;
  }
  .plan{
  /*remove this line: justify-content: space-between;*/
  /*add this line*/
  background-color:/*your color*/
  }
  

  Tip

  Remember to start using relative values rem em instead of absolute values px.

  That's it HAPPY CODING

  Marked as helpful

  0

  This comment has received 2 repliesShow
• 

  raf_0411•160

  @raf0411Submitted over 1 year ago

  I'm having problem with the div won't centered using the display: grid and place-items: center.

  Your feedback would be appreciated...

  

  Stats Preview Card using Grid, Flex, Root, & Pseudo-element

  3

  20
  

  Ed-dyb Abdelilah•1,560

  @eddybproPosted over 1 year ago

  Hi, raf0411

  Congratulation on finishing this challenge, I hope it was fun.

  You did a good job, centering the div you already answered it, you just need to add the min-height property like this:

  • CSS code:

  main{
  min-height: 100vh;
  display: grid;
  place-items: center;
  }
  

  Tha's it, GOOD LUCK IN YOUR JOURNEY

  Marked as helpful

  1
• 

  guptaryan73•150

  @guptaryan73Submitted over 1 year ago

  

  E-commerce product page

  1

  20
  

  Ed-dyb Abdelilah•1,560

  @eddybproPosted over 1 year ago

  Hi, guptaryan73

  Congratulation on finishing this challenge, I hope it was fun.

  Here are some suggestions that might improve the code:

  • HTML code:

  <button class="mobile__nav-trigger">
  <!-It's better to use the 'button' tag instead of the anchor tag in the  'mobile__nav-trigger
  and also for 'anchors' with class: 'mb-row', 'left', 'right', 'lightbox__close-btn'->
  </button>
  

  • CSS code:

  /*The button with the class "lightbox__close-btn" not displaying correctly here is a way to fix it:*/
  .light-box {
  min-height: 100vh;
  padding-top: 5.625rem;
  }
  
  /*The image with the class 'product__img-principal' should have cursor pointer when we hover over it*/
  .product__img-principal:hover{
  cursor: pointer;
  }
  

  Tip

  It's better to use relative values rem em instead of absolute values px which helps makes your website render well in small screen sizes.

  HAPPY CODING

  0
• 

  Priyanshu Pramanik•380

  @Priyanshu-WDSubmitted over 1 year ago

  Hey folks, I have completed this challenge by using picture element to make this responsive for desktop and as well as mobile.

  I am always open to feedback.

  Please let me know if anything lacking in my solution.

  

  Responsive product preview card using picture element.

  2

  60
  

  Ed-dyb Abdelilah•1,560

  @eddybproPosted over 1 year ago

  Hi, PriyanshuPrama12

  Congratulation on finishing this challenge, I hope it was fun. Here are some suggestions to improve the code:

  • CSS code:

  #product button {
  /*update 'width' property/
  width: 100%;
  /*add 'padding' property*/
  padding: .625rem  0;
  /*remove 'height' property*/
  }
  

  Remember to use relative units rem em instead of absolute units px.

  That's it, Happy coding

  Marked as helpful

  0

  This comment has received 1 replyShow
• 

  nina1234567896•280

  @nina1234567896Submitted over 1 year ago

  Any feedback about my projector tips to optimize my project will be appreciated =)

  

  responsive 3 column preview using flexbox

  2

  40
  

  Ed-dyb Abdelilah•1,560

  @eddybproPosted over 1 year ago

  Hi, nina1234567896

  Congratulation on finishing this challenge, I hope it was fun.

  Here are some suggestions that might help improve the code:

  • CSS code:

  main{
  */remove min-width property*/
  width:95%;
  border-radius:0.625rem;
  overflow: hidden;
  }
  
  @media only screen and (min-width: 769px)
  main{
  /*remove that line 'width: 21.875rem;'*/
  }
  p {
  /* remove that line 'width: 15rem;' */
  }
  

  Remember there are also devices with width < 350px, so you should update the media query in CSS .

  /*From this*/
  @media only screen and (min-width: 350px) and (max-width: 768px){}
  /*To this*/
  @media only screen and (max-width: 768px){}
  

  For the units, it's better to use relative units rem em instead of absolute units px.

  That's it, HAPPY CODING

  0
• 

  CocoShesh•160

  @CocoSheshSubmitted over 1 year ago

  1.Are there any issues or improvements you'd suggest for better responsiveness? 2.I'd appreciate any feedback on my code, especially regarding best practices and coding conventions. Are there any areas where you think I could have written the code more efficiently or used better practices?

  

  Proper Styling Using CSS: Cleanliness of the Code

  2

  30
  

  Ed-dyb Abdelilah•1,560

  @eddybproPosted over 1 year ago

  Hi, CocoShesh

  Congratulation on finishing this challenge, I hope it was fun.

  For code here are some suggestions that might help improve the code:

  • HTML code:

  <div class="star-container">  </div>
  

  • CSS code:

  main{
  */ remove the scrollbar in big screen*/
  overflow: hidden;
  }
  .best-tech{
  padding-left:.25rem;
  }
  .star-container {
  width: 6.25rem;
  height: 1rem;
  display: flex;
  justify-content: center;
  background: url(./images/icon-star.svg);
  background-position: center;
  background-repeat: space;
  }
  

  Tip

  The value of the property font-whight does not have a unit.

  If the value of margin, padding is 0 you don't have to add a unit.

  -Examples:

  h1{
  font-whight:700;
  

  p{
  padding-top:0;
  

  I hope my suggestions were helpful.

  Happy coding

  Marked as helpful

  0

  This comment has received 1 replyShow
• 

  Mahmoud_Amin•540

  @Mahmoudamin11Submitted over 1 year ago

  Any feedback please and any advice to reduce the code : )

  

  Responsive order summary card with Flex box

  1

  30
  

  Ed-dyb Abdelilah•1,560

  @eddybproPosted over 1 year ago

  Hi, Mahmoudamin11

  Congratulation on finishing this challenge, I hope it was fun.

  Here are some improvements that might be helpful:

  • CSS code:

  @media (max-width: 430px)
  main {
  /*I think its better to change the value of 'width' to 95%*/
  width: 95%;
  }
  

  It's more professional to add classes to your HTML elements, That makes it easy to identify the element easily in CSS. And you may need to do some tasks later with the class in JS (when the project gets more complex)

  • Example:

  <button class='payment-btn'>Proceed to Payment</button>
  

  You may need to add some hover effect to the button:

  .payment-btn:hover{
  /*some code*/
  }
  

  Tip

  Use relative values rem em instead of absolute values px that help make your website responsive in different screen sizes.

  Good luck in your jouney

  Marked as helpful

  0

  This comment has received 1 replyShow