Skip to content
OverviewSolutions
5
Comments
18
Nguyễn Đức Chính
@chinh1402

All comments

  • Nguyễn Đức Chính•310
    @chinh1402Posted about 1 year ago

    How long did you spend to make this pixel perfect? Cause this is just wild

    1
  • Nguyễn Đức Chính•310
    @chinh1402Posted about 1 year ago

    Congrats on finishing your project 🎉. Your solution looks okay, and I have a few suggestion to improve it

    1. In your code you use padding directly onto the image tag (.card img). That's not really ideal, because by using padding on image, you distorted its border-radius (you can try out yourself by giving it a bigger value of padding). Instead of that, you could: remove all of the padding options in your image, and instead of using width: 80%, and height: 80%, you should change to width: 100% in combination with adding 20px padding onto the card, the qr image would look much better
    2. The spacing between qr-image and headline is too large, I suggest reduce that part a bit; and change the color value to hsl(218, 44%, 22%)

    I hope you find my suggestions useful!

    Happy coding 😊

    0
  • Eric•250
    @erichiraSubmitted about 1 year ago

    Feedback welcome

    QR_Comp

    1
    Nguyễn Đức Chính•310
    @chinh1402Posted about 1 year ago

    Hi there 👋, congrat on finishing the challenge! Here are some changes to your solution you may want to consider:

    1. The spacing between headline and the description text is too close, maybe adding a bit of margin-top or padding-top onto the description texts
    2. The description text is too small, making it hard to read. I suggest increasing the font-size value of the text, maybe 16px is good enough. Those are my takes to improve your solution, what do you think?

    Anywho, happy coding 🤞

    0
  • Nguyễn Đức Chính•310
    @chinh1402Posted about 1 year ago

    Hello there, congrat on finishing the challenge 🎉.Your solution looks good, and I have a suggestion to make it look better, which is:

    Increasing the font-size of headline. Why? The goal of the headline is to be the second thing people look at (the first thing should be the qr-code) to find why they should use that qr-code. By increasing font-size, you are guiding the users to read the UI in a "ordered" way.

    In term of codes, I think you should change a bit in the --spacer variable to make more room for headline text to be bigger (give them more room to work with)

    Hope you find my suggestion useful!

    Happy coding!! 😆

    Marked as helpful
    0
  • erntTt•30
    @erntTtSubmitted about 1 year ago

    I don't really know if there's need of a media queries? I check at a resolution of 375px and was good, what do you think?

    HTML/CSS

    3
    Nguyễn Đức Chính•310
    @chinh1402Posted about 1 year ago

    Hello there ✌, congrat on finishing the challenge. In this specific challenge, there's no need for media queries because their layouts are the same.

    I'll give some suggestions to improve your solution

    1. The space around the image you gave is kind of too big, I suggest shrinking it a bit
    2. There is a lot of white space at the bottom of the card, and there's not enough space between the headline of the card and the image. I suggest moving both headline and description texts down using margin.

    Hopefully you find those suggestions useful.

    Happy coding! 😁

    Marked as helpful
    1
  • Zainabnofiu•80
    @ZainabnofiuSubmitted about 1 year ago

    Hello, this is my third challenge. Please check my code and provide improvements, feedback is very much appreciated

    QR code component

    1
    Nguyễn Đức Chính•310
    @chinh1402Posted about 1 year ago

    Hello, congrats on finishing this challenge 🎉. Your solution looks close to perfect, and I'd love to provide a way to improve it: change the description text color to Grayish blue (in style-guide).

    Why? It's because you're using black color for both headline and description text, which makes both of them fighting for the focal point.

    Hope you find my suggestion useful. Happy coding 😁

    1
  • Sadeesha Jayaweera•240
    @SadeeshaJayaweeraSubmitted about 1 year ago

    It was bit harder when i was trying to add an icon to the button which is shopping cart. Which i haven't done before. However, able to add that after watching a youtube tutorial.

    Please check my code and provide improvements i will be so pleased for your contribution. Thanks a lot.

    Product preview card component

    1
    Nguyễn Đức Chính•310
    @chinh1402Posted about 1 year ago

    Congrats on finishing the challenge 🎉, I have some suggestions for you to improve the design of the card

    1. The challenge has a style-guide, you should follow its layout instruction because when the FEM try to take the image of your solution, it won't look good To actually design on a 1440px layout, You should follow these steps: press f12 to enter inspect mode, then use Ctrl + Shift + M (for Edge). On the top part you will see 2 numbers, Change the first number to 1440px then press enter, Now, you should be able to design on 1440px resolution
    2. The Montserrat font is easily legible, I think it is supposed to be applied on the description part of the page; while Fraunces should be used for the heading... Cause this is a perfume product, Faunces will give that soft and elegent feeling when reading while Montserrat is too bulky.
    3. You should add your card with border-radius to smooth out the edges
    4. The button has an icon that is too close to the text. You should add some more margin for them

    Hope you find my suggestions useful

    Happy coding ✌

    Chinh.

    Marked as helpful
    0
  • Nate Poli•60
    @njpoliSubmitted about 1 year ago

    What did you find difficult while building the project?

    • This was my first time using tailwind and next.js with the new app router.
    • I spend a decent amount of time reading the docs for project layout/installation even though this project was pretty simple.

    Which areas of your code are you unsure of?

    • I did place the code section in the provided project files as a "footer" here:
    function Footer() {
  return (
    <div className="sticky bottom-0 text-center">
      Challenge by{" "}
      <a href="https://www.frontendmentor.io?ref=challenge" target="_blank" className="font-bold underline hover:decoration-2">
        Frontend Mentor
      </a>
      . Coded by <a href="https://www.github.com/njpoli" className="font-bold underline hover:decoration-2">njpoli</a>.
    </div>
  );
}

    I placed my footer in the page.tsx file like so:

    export default function Page() {
  return (
    <>
      <div className="flex flex-col justify-center items-center h-screen">
        <QRCode />
      </div>
      <Footer />
    </>
  );
}

    However, this created a small scroll bar even though the footer is correctly sticking to the bottom of the viewport. When I place the footer inside of the div with the QRCode component, it doesnt go to the bottom of the viewport, it is directly under the QRCode component. How can I have the footer stick to the bottom without creating a scrollbar?

    Do you have any questions about best practices?

    Should the footer contain the sticky css property or does that belong in the parent component? Any other tips/suggestions welcome as well.

    First solution using next.js and tailwindcss.

    1
    Nguyễn Đức Chính•310
    @chinh1402Posted about 1 year ago

    Hello ✌, well done on finishing the challenge. The scrollbar appeared because you used "h-screen" class which gave the whole card part span 100% viewport height, and therefore, the attribution part is going to make more space. And even with position: sticky, it still stays as a part of the website flow and occupies more space.

    My solution for this is to give the position of attribution part absolute or fixed, that way, it won't stay as a part of the website flow, and won't create a scrollbar like so:

    .absolute {
    position: absolute;
   / * centering the attribution texts */
    left: 50%;
    transform: translateX(-50%);
}

    Hope you find my comment useful 😁

    Marked as helpful
    1
  • Nguyễn Đức Chính•310
    @chinh1402Posted about 1 year ago

    Okay, a lot of things I think I gotta point out here.

    1. The image is inside the div with the class "container", but it is expanded out of "container" because the image itself is too big. Instead of giving inline-style of width: 300px, I suggest giving it width: 100%; That way, the image will fill up the width of the content part of the container, and won't actually make its way out of the container div
    2. The challenge's design has some nice rounded corners, you want that to be on your design as well. You should do some research on "border-radius" property, it would make your design much more satisfying
    3. I suggest you use the font from the style-guide of the challenge; The way I usually use the font is to use links embed from google font, you should do some research on that too
    4. The way you centered the card is not ideal.. There are a lot of ways for you to center anything. I'll provide you with a way that I usually do, you can check my codepen

    If anything you want to ask, you can reply directly under this comment, and I'll try to answer them to the best of my knowledges

    Everyone has a starting point. Hopefully, I can see more of your solution in the future.

    Happy coding 🤞

    Marked as helpful
    0
  • Nguyễn Đức Chính•310
    @chinh1402Posted about 1 year ago

    Hello 👋, well done on finishing the challenge. Your design looks great overall, but here are my ideas to make it look better

    1. The description text is a bit too wide, it's too close to the side of the card. You should give it more padding for it to work with. Same thing with your headline text
    2. The padding you gave for the image is inconsistent. The padding on top is too big while the padding on the sides are too small. You should adjust it, 20px for all sides is a good fit here.
    3. The rounding of borders is inconsistent as well. The outer card should be rounded more; while the image border, the top part is too small, while the bottom part is too big; As I checked your code, the padding-top of the image just somehow distorted the top part border-radius, which is kind of unfortunate. My work-around for this is: remove all the padding of the image and, instead, add padding onto the card that wrap around the image with that white background, and now you got a nice, lovely image with no distorted border-radius

    With those fixes, I think your solution would look much better. What do you think?

    Marked as helpful
    1
  • Nguyễn Đức Chính•310
    @chinh1402Posted about 1 year ago

    Hello 👋, well done on finishing the project. This is well made, and here are my opinions on your solution.

    1. The description text is a bit too big, it is kind of taking up the focal point of the image, How about trying a smaller font-size
    2. The space between the description text and the bottom edge of the image is too close, the design would look cleaner if you give it more space to work with
    3. A bit nit-picky, but I don't really fond of the pure black colour of headline. You should use the lighter one, probably using the dark blue which was given in the style guide of the challenge

    Overall, your solution is good enough. Those above suggestions are just ideas to improve, hope you find them useful

    Cheers, and happy coding 🤞

    0
  • Khánh•160
    @hnvkhanhSubmitted about 1 year ago

    I've encountered a challenge in altering the input border while it's in focus. I tried focus:border-purple but it does not work. purple is my custom color in Tailwind. Also, which is the best way to make the animation in the calculated age (which is a bonus of this challenge)?. Thank you all for reviewing my solution :3.

    Age Calculator App with ReactJS and Tailwind CSS

    2
    Nguyễn Đức Chính•310
    @chinh1402Posted about 1 year ago

    Just a suggestion, Did you try :focus and outline: none to alter input border while it's in focus?

    You can have a look here

    Marked as helpful
    2