Skip to content
OverviewSolutions
36
Comments
133
Kimo Spark
@kimodev1990

All comments

  • Kimo Spark•2,190
    @kimodev1990Posted 10 months ago

    Really great work in completing the challenge and loved the transition of colors while hovering, Just a simple feedback :

    • If I added space at start or end of the input email ( not in between ), It gives an error. To eliminate this issue , You could just adjust in the javascript :
    form.addEventListener("submit", function (e) {
      e.preventDefault();

      if (!emailCorrectFormat(emailInput.value.trim( ))) { rest of your code }

    and

    state = "success";
email = emailInput.value.trim( ) ;
rest of your code

    The rest of your code is okay, trim ( ) removes if there is any spaces added in input.

    Hope you find this Helpful.

    Other than that, Nice work & Keep going on.

    Marked as helpful
    1
  • Kimo Spark•2,190
    @kimodev1990Posted 10 months ago

    Great work in completing the challenge, Just a few feedbacks :

    • It better ( here & future designs ) to assign percentage values instead of definite values, such as you could assign width of image and class .content width: 50% ; for each of them, so you it would be in relevant to the main container's width and having better responsiveness.
    • You could center your design in middle of the website by adding in body :
    display: flex ;
justify-content: center ;
align-items: center ;
min-height: 100vh ;

    then container under body will be centered, no need to use top: 120px ; & left: 28% ;

    • Wrap the page's whole content in the <main> tag.
    • Use font-size in rem or em units rather than pixels for better responsiveness
    • You could use clamp ( ) method in your coding for font-size, width, margins, padding, etc., So the designed sizes will change according to the viewport dimensions having a responsive design and will be suitable for any device layout.

    Hope you find this Helpful.

    Other than that, Really Nice work & keep Going on.

    Marked as helpful
    0
  • Kimo Spark•2,190
    @kimodev1990Posted 10 months ago
    • Use rem or em units rather than pixels for better responsiveness
    • You could use clamp ( ) method in your coding for font-size, width, margins, padding, etc., So the designed sizes will change according to the viewport dimensions having a responsive design and will be suitable for any device layout.

    Hope you find this Helpful.

    Other than that, Really Nice work & keep Going on.

    Marked as helpful
    0
  • Kimo Spark•2,190
    @kimodev1990Posted 10 months ago
    • You could use clamp ( ) method in your coding for font-size, width, margins, padding, etc., So the designed sizes will change according to the viewport dimensions having a responsive design and will be suitable for any device layout.

    Hope you find this Helpful.

    Other than that, Really Nice work & keep Going on.

    Marked as helpful
    0
  • Kimo Spark•2,190
    @kimodev1990Posted 10 months ago

    Really Great work in completing the challenge, Just a few feedbacks :

    • To center your container, no need to use padding: 175px 420px in container, You could add in body :
    display: flex ;
justify-content: center ;
align-items: center ;
min-height: 100vh ;

    and the container under body will be centered in the middle of your website.

    • You could add background-color: var(--cream-bg) to body instead of container to cover the whole page similar to the desired design.
    • Other way to change images according to the design layout is to use picture tag :
    <picture>
      <source media=" ( max-width:991.5px ) " srcset="./images/image-product-mobile.jpg>
      <img  src="./images/image-product-desktop.jpg" alt="#">
</picture>

    and depending on @media, the image changes. No need to add display: none ;

    Hope you find this Helpful.

    Other than that, Really Nice work & keep Going on.

    Marked as helpful
    0
  • hakim taibi•50
    @taibihakim2002Submitted 10 months ago

    It is a simple and easy challenge, I didn't face any difficulties But I am not sure about the solution to the application in best practices, If you have any comments about this, let us know Thank you ♥

    Newsletter Sign Up Form With Success Message

    1
    Kimo Spark•2,190
    @kimodev1990Posted 10 months ago

    Really great work in completing the challenge and loved the transition between two containers, Just a simple feedback :

    • If I added space at start or end of the input email ( not in between ), It gives an error. To eliminate this issue , You could just adjust in the javascript :
    e.preventDefault( );
let emailValue = emailInput.value.trim( ) ;

    The rest of your code is okay, trim ( ) removes if there is any spaces added in input.

    Hope you find this Helpful.

    Other than that, Nice work & Keep going on.

    0
  • programmeur77•100
    @programmeur77Submitted 10 months ago

    Hi everybody,

    I just completed that challenge. The only question I have is the following :

    • How can I put the second image - the one for mobile -, for the responsive layout, whereas the first one is hard coded in HTML folder

    Product preview card

    1
    Kimo Spark•2,190
    @kimodev1990Posted 10 months ago

    Great work in completing the challenge, Just some feedbacks:

    • As for your question, You could picture tag to move between two pictures depending on viewport dimension layout. For example: Our mobile layout design starts from viewport width of 375px and above that is desktop layout design . So we could use :
    <picture>
      <source media=" ( max-width:375.5px ) " srcset="./images/image-product-mobile.jpg>
      <img  src="./images/image-product-desktop.jpg" alt="#">
</picture>

    You shall realize that in desktop layout, the primary image will appear and when the viewport dimensions decreases to 357 px in width ( mobile devices ), the image changes to image-product-mobile.

    • For your article class .product-card , It's better to make it flex instead of grid, The reason for that is because in desktop layout flex-direction: row ; and in mobile layout you could easily stack the image and details above each other flex-direction: column ;
    • I've realized the design doesn't change between desktop and mobile layouts, You should use @media ( media query ) to change your layout design and make it responsive.
    • No need to assign grid to body, If you've assigned it to center your design, you could add :
    display: flex ;
justify-content: center ;
align-items: center ;
min-height: 100vh ;

    And main class .product-card-container will be centered in the middle of your website.

    Hope you find this Helpful.

    Other than that, Really Nice work & keep Going on .

    0
  • cpylypow•30
    @cpylypowSubmitted 10 months ago

    hello everyone what do you think of the project so far? what changes would you suggest going forward thanks

    QR Code Component

    1
    Kimo Spark•2,190
    @kimodev1990Posted 10 months ago

    Really Great work in completing the challenge, Just a few feedbacks:

    • Wrap the page's whole main content in the <main> tag.
    • Starting from 400 px in layout's width for viewport dimensions and decreasing, You might notice your font-sizes becomes larger and isn't responsive for smaller layouts ( mobile devices ). For making your design more responsive ( here & future designs ) , You could use clamp ( ) method in your coding for font-size, width, margins, padding, etc., So the designed sizes will change according to the viewport dimensions having a responsive design and will be suitable for any device layout.

    Hope you find this Helpful.

    Other than that, Really Nice work & keep Going on

    Marked as helpful
    0
  • Kimo Spark•2,190
    @kimodev1990Posted 10 months ago

    Great work in completing the challenge, Just a simple feedback :

    • You could use clamp ( ) method in your coding for font-size, width, margins, padding, etc., So the designed sizes will change according to the viewport dimensions having a responsive design and will be suitable for any device layout.

    Hope you find this Helpful.

    Other than that, Really Nice work & keep Going on

    Marked as helpful
    0
  • Kimo Spark•2,190
    @kimodev1990Posted 10 months ago

    Great work in completing the challenge, Just a few feedbacks :

    • To center your design, You could add in body :
    display: flex ;
justify-content: center ;
align-items: center ;
min-height: 100vh ;

    and no need to add margin in div class .card

    • Wrap the page's whole main content in the <main> tag.
    • You could assign width & height of image with percentage values instead of definite values for better responsiveness. For example , for width of image could be width : 90% ; instead of width: 250px ;

    Hope you find this Helpful.

    Other than that, Nice work and Keep going on.

    0
  • Dimitri•680
    @DimitriTsikaridzeSubmitted 10 months ago

    I couldn't figure out how to add smooth transitions with grid template rows.

    FAQ accordion

    1
    Kimo Spark•2,190
    @kimodev1990Posted 10 months ago

    Really great work in completing the challenge, As for smooth transitions as the way I did it :

    • It's better for active tag to be flex.
    • Wrap the <p> tag which contains the the answer under Div and assign a class for it for example called .answer-container .
    • In CSS
    .answer-container {
    max-height: 110px;
    transition: max-height 0.5s;
    overflow: hidden;
}

    and will add new class next to previous class :

    .answer-Text {
    max-height: 0px;
}
    • In Javascript :
    var toggleButton = document.getElementsByClassName("Class you've assigned to the button image");
for (let i = 0; i < toggleButton.length; i++) {
  toggleButton[i].addEventListener("click", () => {
    var answer = document.getElementsByClassName("answer-container");

    if (answer[i].classList.contains("answerText") === true) {
      answer[i].classList.remove("answer-Text");
    } else {
      answer[i].classList.add("answer-Text");
    }
}

    Any Questions, Don't hesitate to ask.

    Hope you find this Helpful.

    Other than that, Nice work & Keep on the great work.

    0
  • Kimo Spark•2,190
    @kimodev1990Posted 10 months ago

    Really great work in completing the challenge, Just a few feedback :

    • Wrap the page's whole content in the <main> tag.
    • For your div class .card-img & div class .card-body , Intstead of assigning them width: 300px ; , It's better to assign percentage values such as width: 50% ; , So you could have a responsive design in relative with the size of div class .card .
    • Use font-size in rem or em units rather than pixels for better responsiveness .
    • I'd like to suggest to check up clamp ( ) method. You could use clamp ( ) method in your coding for font-size, width, margins, padding, etc., So the designed sizes will change according to the viewport dimensions having a responsive design and will be suitable for any device layout.

    Hope you find this Helpful.

    Other than that, Really Nice work & keep Going on .

    Marked as helpful
    0