Skip to content
OverviewSolutions
9
Comments
9
Nadun Dhananjaya
@nadun-dhananjaya

All comments

  • Sebastian•290
    @SebBudynskiSubmitted 11 months ago

    Hey, It's my first project on Frontendmentor where i used JavaScript. I'm very happy of progres i'm making by doing this exercises. Happy to hear what i can improve in my code.

    Regards Sebastian

    Accordion flexbox JS

    4
    Nadun Dhananjaya•210
    @nadun-dhananjayaPosted 11 months ago

    Hello 👋. Congratulations on successfully completing the challenge! 🎉

    I have additional recommendations regarding your code that I believe will greatly interest you.

    • I noticed that you had set body { max-width: 38px }. Setting a max-width for the body is not a good practice. Instead, you can add a child element and set a max-width for that child element.

    • According to your HTML code structure, you can set a max-width for the main element.

    CSS Changes :

    body {
  background-image: url(assets/images/background-pattern-desktop.svg);
  background-color: var(--Light-pink);
  background-repeat: no-repeat;
  background-size: 100%;
  font-family: "Work Sans", sans-serif;
  min-height: 100vh;
  display: flex;
  align-items: center;
}
main{
  max-width: 38rem;
}
    • Now you have to center the according card. But if you use
      display: flex;
  align-items: center;
  justify-content: center;

    • it compresses its child element as much as possible. and when you expand the question, the card width will increase. As a solution, you can set a fixed width for the main tag. But the issue with that is making it responsive is a challenge for you.

    • Therefore, we can use position: absolute instead of flex box.

    body {
  background-image: url(assets/images/background-pattern-desktop.svg);
  background-color: var(--Light-pink);
  background-repeat: no-repeat;
  background-size: 100%;
  font-family: "Work Sans", sans-serif;
  min-height: 100vh;
}

main {
  position: absolute;
  transform: translateX(-50%);
  top: 30%;
  left: 50%;
  max-width: 40rem;
  width: 100%;
  padding: 0 1rem;
}

.content {
  background-color: white;
  border-radius: 0.6rem;
  padding: 1.5rem;
  width: 100%;
}

@media screen and (max-width: 1280px) {
  main {
    top: 18%;
  }
}
@media screen and (max-width: 768px) {
  main {
    top: 10%;
  }
}

@media screen and (max-width: 450px) {
  main {
    top: 7%;
  }
}
    • Here you can see that I have set specific top positions for different screen sizes. Because if we place It in the center of the screen when we expand the question, it expands both the top and bottom of the card. It's not a good visual experience here. If you set specific top positions, the card will only expand from the bottom. That's better.

    I hope it was helpful, you are great, keep up the good work 👍

    Happy coding! 😎😎😎

    Marked as helpful
    0
  • Nadun Dhananjaya•210
    @nadun-dhananjayaPosted 11 months ago

    Hello 👋. Congratulations on successfully completing the challenge! 🎉

    I have other recommendations regarding your code that I believe will greatly interest you.

    HTML and CSS

    • I recommend enhancing the structure of your HTML document by incorporating a div tag to encapsulate the card content within a container, allowing for better organization and improved browser understanding. This can be achieved by creating a container div with a class of "container" to house the entire content.
    <html lang="en">
  <head>
    <-- Your head content here -->
  </head>
  <body>
      <-- container -->
      <div class="container">
         
     </div>
 </body>
</html>
    • Within this container, you can include your a header section and main section of your newsletter card.
    <html lang="en">
  <head>
    <-- Your head content here -->
  </head>
  <body>
      <-- container -->
      <div class="container">
          <header>
                <-- Your head content here -->
          </header>
         <main>
                <-- Your main content here -->
          </main>
     </div>
 </body>
</html>
    • To ensure a visually appealing layout, apply the following CSS styles to the container class.
    .container {
    display: flex;
    flex-direction: row-reverse;
}

    Additionally, style the body tag with CSS to ensure the container is positioned at the center of the screen.

    body {
    min-height: 100vh;
    padding: 2rem;
    background-color: var(--DarkSlateGrey);
    display: flex;
    justify-content: center;
    align-items: center;
}

    I hope it was helpful, you are great, keep up the good work 👍

    Happy coding! 😎😎😎

    Marked as helpful
    0
  • Nadun Dhananjaya•210
    @nadun-dhananjayaPosted 11 months ago

    Hello 👋. Congratulations on successfully completing the challenge! 🎉

    I have other recommendations regarding your code that I believe will greatly interest you.

    HTML and CSS

    • In this challenge Newsletter sign-up card is the main content of the page. Therefore, adding a <main> tag around the container is a good practice.
    <html lang="en">
  <head>
    <-- Your head content here -->
  </head>
  <body>
    <-- main -->
    <main>
      <-- container -->
      <div class="container">
         <-- Card content here -->
     </div>
  </main>
 </body>
</html>
    0
  • Nadun Dhananjaya•210
    @nadun-dhananjayaPosted 11 months ago

    Your work is excellent, especially the animation.

    0
  • Nadun Dhananjaya•210
    @nadun-dhananjayaPosted 11 months ago

    Hello 👋. Congratulations on successfully completing the challenge! 🎉

    I have other recommendations regarding your code that I believe will greatly interest you

    HTML and CSS

    • In this challenge Blog card is the main content of the page. Therefore, adding a <main> tag around the container is a good practice.
    <html lang="en">
  <head>
    <-- Your head content here -->
  </head>
  <body>
    <-- main -->
    <main>
      <-- container -->
      <div class="container">
         <-- Card content here -->
     </div>
  </main>
 </body>
</html>
    • The Blog card is not in the middle of the screen. You can centralize your blog card by centralizing the items inside the main tag.

    You can centering the card on the screen using display: gird:

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

    or

    using display: flex as below

    main {
    display: flex;
    justify-content: center;
    align-items: center;
    min-height: 100vh;
}
    0
  • Nadun Dhananjaya•210
    @nadun-dhananjayaPosted 11 months ago

    Congratulations for completing challenge

    0
  • Nadun Dhananjaya•210
    @nadun-dhananjayaPosted about 1 year ago

    great job !

    1
  • Nadun Dhananjaya•210
    @nadun-dhananjayaPosted about 1 year ago

    Rating: ★★★★★

    I recently had the opportunity to experience your website, and I must say that overall, it's been a great experience. The content, design, and usability are all praiseworthy. However, while exploring it on mobile screens, I noticed a minor issue that I believe could be improved for an even better user experience.

    The problem I encountered is that the Call to Action (CTA) section on mobile screens appears too close to the footer.

    To address this issue, I'd like to suggest a couple of methods that you can consider:

    1. Adding Padding to the CTA Section: You can add a padding-bottom property to the CTA section when the screen width is less than 800px. This will create some separation between the CTA and the footer, making it more visually appealing and easier for users to interact with the CTA.

     @media screen and (max-width: 800px) {
       .cta {
           padding-bottom: 3rem;
       }
   }

    2. Adding Padding to the Last Child of the Courses Container: Another approach is to add a padding-bottom property to the last child of the courses container when the screen width is less than 800px. This will introduce space between the last course and the footer, addressing the issue while maintaining consistency with your design.

     @media screen and (max-width: 800px) {
       .courses:last-child {
           padding-bottom: 3rem;
       }
   }

    These solutions should help create a better visual hierarchy and user experience, particularly on mobile screens. Overall, you've done a great job with your website, and addressing this minor issue will make it even better. Keep up the good work!

    I hope these suggestions are helpful, and I look forward to seeing the improvements on your website.

    Marked as helpful
    0
  • Dinael Domingos Santos•40
    @DinaelDomingosSubmitted over 1 year ago

    This is a solution for the NFT preview card component challenge in Frontend Mentor](https://www.frontendmentor.io/challenges/nft-preview-card-component-SbdUL_w0U). This is my second challenge and they have helped me a lot to improve my HTML and CSS skills.

    I tried to adopt semantic HTML5 markup, CSS custom properties and used Flebox in almost all DIV elements.

    My biggest difficulty was with the overlay effect for DIV .image . I didn't remember how to apply the effect and it was necessary to research other ready-made solutions to be able to deliver the final project. But in the end it was a great learning experience.

    Please feel free for any comments and feedback.

    NFT preview card component

    2
    Nadun Dhananjaya•210
    @nadun-dhananjayaPosted over 1 year ago

    Hello 👋. Congratulations on successfully completing the challenge! 🎉

    I have other recommendations regarding your code that I believe will greatly interest you

    HTML and CSS

    • The NFT card is not center align

    You can centralize your NFT card by centralizing the items inside the main tag.

    You can center the items inside the main tag using display: gird

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

    or

    using display: flex as below

    main{
    display: flex;
    place-items: center;
    height: 100vh;
}
    Marked as helpful
    0