Skip to content
OverviewSolutions
25
Comments
40
Hikmah
@Hikmahx

All comments

  • Hikmah•1,070
    @HikmahxPosted over 1 year ago

    Hi CHarvey 👋! You did excellent work on this project! I have a suggestion for you:

    For the box class, it would be better if the width is set to 100% rather than a fixed 450px. This is because when an element has a max-width property, the width becomes responsive on smaller devices and stops increasing beyond that specific max-width.

    I hope this helps

    Marked as helpful
    0
  • Hikmah•1,070
    @HikmahxPosted over 1 year ago

    Hi Lipe 👋! I checked your solution and you did great work on this project! I have a few suggestions for you:

    • You should add a padding of 1rem to the body of your application:
    body {
    padding: 1rem;
}

    This is so that on smaller screen sizes, the QR code container can have some spacing around it.

    Hope this helps

    Marked as helpful
    0
  • Hikmah•1,070
    @HikmahxPosted almost 2 years ago

    Hi sirgra 👋! I checked your solution and you did great work on this project! I have a few suggestions for you:

    • Since it is a single component, the card should be centered and the page should be the size of the device. This means the project should simply occupy the whole height. To do this, replace the margin in the parent-container with margin: auto and remove the media query because margin: auto is all you'd need. This will make the card centered.
    • To the body of the code, add this:
    body {
    height: 100vh;
    display: flex;
    flex-direction: column;
}

    The height: 100vh; will make the height of the page the full height of the device, making it a full page. using display: flex; will shift the card to the center vertically and since by default, the file direction is row, ie the element in the body will be arranged in the horizontal row, adding flex-direction: column makes the card and attribution to be organized in a vertical way.

    Hope this helps

    Marked as helpful
    1
  • Hikmah•1,070
    @HikmahxPosted almost 2 years ago

    Hi @ubonisrael 👋! I check your website and you did a great job with this solution. The pages are highly responsive and everything looks good.

    I noticed that you could enhance the user experience by adding a few transitions to the slider and tabs. For instance, you could consider adding a fade-in effect when a new tab is clicked while the former fades out. This would make the browsing experience smoother and more enjoyable for your users.

    Marked as helpful
    1
  • Hikmah•1,070
    @HikmahxPosted almost 2 years ago

    HI Safi 👋! Great job!

    I tried viewing the code on your GitHub so I can give you a suggestion but I couldn't find the JavaScript. I wanted to rewrite the code based on how you write yours.

    This is just a simple suggestion, something you may use.

    So for the border countries, if you want to get the full country name instead of the abbreviation, you can fetch the border countries inside the function you use to get a single country detail.

    async (name) => {
    try {
      let { data } = await axios.get(
        `https://restcountries.com/v3.1/name/${name}?fullText=true`
      );
      const country = await data;

       // CODE IS HERE
      const borders = await data[0].borders;
      if (borders) {
        let borderDetails = await axios.get(
          `https://restcountries.com/v3.1/alpha?codes=${borders.join(",")}`
        );

        let allBorders = await borderDetails.data.map(
          (border: any) => border.name.common
        );

        return { country, allBorders };
      }
      return { country };

      // return allBorders;
    } catch (err) {
      return err
    }

    Hope this helps

    Marked as helpful
    0
  • Hikmah•1,070
    @HikmahxPosted almost 2 years ago

    Hi @Mirage9898 👋. Nice work! I have a few suggestions for your solution:

    • I think adding a margin to the container, like margin: 16px;, will be better for mobile view, to add a bit of spacing around the container.
    • For the two images, icon-ethereum.svg and icon-clock.svg, not to give a stretched appearance, adding object-fit: contain; will allow these images to retain their shapes.

    Hope this helps

    0
  • Hikmah•1,070
    @HikmahxPosted almost 2 years ago

    Hi Santiago! For the animation, I used Countupjs, the react package. I searched online and there is also a component for Vuejs you can try out in npm.js.

    Marked as helpful
    1
  • Hikmah•1,070
    @HikmahxPosted almost 2 years ago

    Hey Matheus 👋! Everything looks good. I have just one piece of feedback:

    • To solve the overflowing body, you can add overflow:hidden; to the section with the class banner and the footer. And the scroll bar will disappear
    0
  • Hikmah•1,070
    @HikmahxPosted almost 2 years ago

    Hi Talisson 👋, Nice work there! Tailwind can be a bit overwhelming at first, especially as a newbie. I remember not wanting to use it but because I kept seeing it everywhere, I decided to give it a try. And I don't regret it. Almost all my projects use Tailwind and I'll continue to use it in the future. That being said, I have some tips for you:

    • I think it'll be better to add some space at the top and bottom of the card in smaller devices, perhaps of the tailwind class py-4 in the div immediately after the main.
    • The title should be smaller on mobile devices, like text-3xl, and prefix the class text-5xl with md: (tablet) or lg: (desktop).
    • For checking accessibility, you can install the Chrome extension Wave. I recently discovered this and it's very useful. Just right-click on the page after installing Wave and you can wave the page.

    Hope this helps you

    Marked as helpful
    1
  • Hikmah•1,070
    @HikmahxPosted almost 2 years ago

    Hi Alexander. Great solution! There are a few feedbacks that may help with your site:

    • I think you should use a single-column grid or maybe flex display when on smaller screens like mobiles and tablets, and change it back to the 3-column grid when the media query is 1024px min-width. Adding a max-width property makes sure the card doesn't over-stretch and the margin aligns everything to the center.
    .cuadros {
    grid-template-columns: repeat(1, 1fr);
     max-width: 500px;
    margin: auto;
}

    And you can also simply reset the max width back to none, max-width: none;, when on 1024px.

    Hope this helps

    Marked as helpful
    1
  • Hikmah•1,070
    @HikmahxPosted almost 2 years ago

    Hi @CriKno. Great work! I have some feedbacks that may help improve this project:

    • For the card, instead of setting the width to 336px and then 579px on tablet, I think it is better to replace it with max width and set the width to 100%:
    .card {
    width: 100%;
    max-width: 336px;
}

@media (min-width: 640px){
    .card {
        max-width: 579px;
    }
}

    This is will make the card more responsive. You may add a little padding to the main tag to give some spacing at the side of the card:

    main {
    padding: 16px;
}

    I hope this helps

    Marked as helpful
    1
  • Hikmah•1,070
    @HikmahxPosted almost 2 years ago

    Hi Joshua! I have two little feedbacks that may help improve your site:

    • For the advice generate text to be centered rather than aligned to the left, you can add text-align: center; so that when the text is longer than a line, it is aligned to the center. Since you are using tailwind, you can simply add text-center class to the element of id advice
    • I think using the max-width property is better than the class md:w-2/5. You replace that class using the tailwind's max-width: max-w-xl
    Marked as helpful
    0
  • Hikmah•1,070
    @HikmahxPosted almost 2 years ago

    Hey Victoria 👋, I just wanted to start by saying a great job on everything! I have some feedback that I think could be helpful:

    • I was thinking that for the 'Change' and 'Cancel order' text, it might be better to use a button tag instead of an h2 tag. Since they're positioned like buttons and seem to be clickable, using button tags could make it clearer for users that they can interact with them, and it could also improve accessibility.

    To make the website more responsive:

    • Add a bit of padding to the body so that there can be space around the container when in mobile view:
    body {
    padding: 1rem;
}
    • For the container, add width and max-width so the container doesn't overstretch, and also as the screen width increases, the container stops at the initial width you made it before:
    .container {
    max-width: 450px;
    width: 100%;
}
    • Add a width:100% to the img tag of id illustration so it also becomes responsive with the container.
    • In the div of id price, change justify-content to space between so the elements in it shift to both ends with a flex-wrap for smaller screen devices so the div doesn't overflow in smaller devices:
    #price  {
    justify-content: space-between;
    flex-wrap: wrap;
}

    You can change the margin-right to auto in the element of id info to make this element stay at the left. This is because when you add justify-content: space-between; to the price parent element, it stays in the middle.

    Hope this helps

    Marked as helpful
    0
  • Hikmah•1,070
    @HikmahxPosted almost 2 years ago

    Hi Yousef. Great work! I have a suggestion to make all the cards centered in desktop view. To the container, instead of using grid-template-columns: repeat(auto-fill, minmax(300px, 1fr));, replace it with grid-template-columns: 300px 300px 300px; for a 3-column grid and also justify-content: center; to center them.

    Hope this helps

    0
  • Hikmah•1,070
    @HikmahxPosted almost 2 years ago

    Hi Jonathan 👋. Congrats on your first React project! I checked your live link and just wanted to say everything looks fine from my end. Nice works!

    1
  • Hikmah•1,070
    @HikmahxPosted almost 2 years ago

    Hi Abork 👋. Nice work! I have some suggestions for you:

    • For the pattern divider svg when in mobile view, it's better to change the original width, width=250px, to the max-width instead. The reason is that in smaller devices the image overflows to the right. By changing the width to 100%, it makes sure it doesn't overflow, and adding a max-width property will make the width stop increasing on larger devices like tablets. Setting the margin to auto helps center the image when it reaches its max width.
    .mobile-img {
    width: 100%;
    max-width: 250px;
    margin: auto;
}
    • For the main container, it is better instead of setting the width to 25% to give it a width and max-width property like the above. This way the container doesn't become stretched as the screen width increases.
    .main-container {
    width: 100%;
    max-width: 520px;
}

    Hope this helps

    Marked as helpful
    0
  • Hikmah•1,070
    @HikmahxPosted almost 2 years ago

    Hi Hassan 👋. Great solution! I have one suggestion. I noticed when checking the website, after the breakpoint of 500px, it sort of acted up. I was thinking maybe you should increase the max-width of the breakpoint from 500px to perhaps 800px or so to make the site more responsive. Hope this was helpful

    Marked as helpful
    1
  • Hikmah•1,070
    @HikmahxPosted almost 2 years ago

    Hi @Petrosdevri 👋. Nice work! I have some suggestions for you:

    • For the image, pattern-divider.svg, not to overflow on smaller devices, it is better to add width and max-width property:
    .advice-text {
    img{
        width: 100%;
        max-width: 300px;
   }
}

    By adding the width, it contracts and expands based on the screen size. Adding a max width will make it not expand way beyond the width you initially wanted. You can set the max-width above to whatever you want.

    • It is better to add padding to the advice-app class because as the screen gets smaller, the spacing at the side disappears:
    .advice-app {
    padding: 16px;
}

    Hope this was helpful

    Marked as helpful
    0