Skip to content
OverviewSolutions
10
Comments
12
nerometa
@nerometa

All comments

  • nerometa•280
    @nerometa

    Wow! Great job of you to make this looks very close to the design. A few recommendations, though:

    • The HTML of the price and time section. It looks kinda odd to me when you put an <img> tag inside <p> tag, which mainly uses for texts. I think you can improve upon it by grouping these elements together with <div>, then use <img> for icon and maybe <span> for text so it begins on the same line as your icon. Something like this:
    <div>
<img src="ethereum.svg" alt="Ethereum">
<span>0.041 ETH</span>
</div>
    • This is the matter of taste, but I noticed that any links on the card (like the NFT name or the author's name), you tend to put an <a> tag over any text element to make it resembles a link. I kinda get it. Personally, I would put an <a> tag inside text element like:
    <span>I would like to introduce you to this <a href="#">hyperlink</a>.</span>

    That's my two cents. You're doing great!

    Marked as helpful
  • nerometa•280
    @nerometa

    This is truly amazing, Lucas! Love the hover effect and your take on color that's entirely different to the design.

    After seeing this, I HAVE to follow you man. You're that amazing😀

  • nerometa•280
    @nerometa

    Hmm, looks like your colors are a bit off. But don't worry, it's pretty easy for you to do and I know you can do it.

    in your tailwind.config.js file, you can add custom colors from the style guide that's been provided with the challenge like this:

    theme: { extend: { colors: { "custom-color": "#FF0000" } } }

    and to use it as a utility class, you can then do bg-custom-color or text-custom-color since you've put your color in your config file, Tailwind will know.

    Also, good work! I wanna see you improve this!

  • nerometa•280
    @nerometa

    You're doing great so far! Here's one of the solution I use for your image issue:

    You make 2 <img> tags with different images together, maybe assign a class to work with CSS later. Suppose you got one <img> with mobile image and another <img> for desktop image. Then, for it to change, you put all of that <img> into @media queries and set property display: none when you want to hide it and set display: block for an image you want to show.

    Suppose you got your images and you decided that 720px will be your breakpoint. Set that @media query to (max-width: 720px), which means anything that's below 720px will show up (smaller devices). You might put your desktop image to display: none and mobile image to display: block.

    You don't have to do another @media (min-width: 720px) for desktop since ANYTHING that's not in media query works similar to that. So, out of @media query, you set your desktop image to display: block and then your mobile image to display: none

    Other than that, great job! Keep coding✌⌨

  • nerometa•280
    @nerometa

    ทุกอย่างโอเคเลยฮะ responsive ก็ทำได้ดีเลย ทีนี้ผมอยากให้ลองอะไรหน่อย

    • ตั้งค่า <body> ให้มีความสูงเท่าขนาดหน้าจอ (height: 100% หรือ height: 100vh)
    • ทำ wrapper <div> ครอบตัวคอนเทนต์หลักอีกทีนึง กำหนดให้ทั้งความกว้างและความสูงขนาดเต็มหน้าจอเหมือนกัน
    • wrapper ตัวนั้นให้ display เป็น flex, justify-content เป็น center เพื่อจัดตำแหน่งตามแนวนอน แล้ว align-items: center เพื่อจัดตำแหน่งตามแนวตั้ง แล้วตัวคอนเทนต์หลักจะอยู่กลางจอพอดีฮะ
  • nerometa•280
    @nerometa

    Like others say, it seems like you didn't include images folder into your project. Make sure you push it into Github so your image will show. Add an 'alt' for more accessibility.

    I'm not certain about filepath. maybe "./images/image.png" works, maybe "images/image.png" works depends on where you deploy your app, I think.

    Marked as helpful
  • nerometa•280
    @nerometa

    ค่อนข้างตรงดีไซน์เลยฮะ ถือว่าทำได้ดีมากสำหรับโปรเจคท์แรก ผมมีข้อแนะนำอยู่ประมาณนึง คิดว่าคุณต้องแก้ไขได้แน่ๆ เลยฮะ

    1. การเอา .svg เข้ามาใช้ เราใช้แท็ก <img> ได้ครับ เช่น <img src="assets/little-icon.svg"> จะประหยัดโค้ดมากขึ้นครับ
    2. การจะทำ wrapper อันใหญ่ครอบตัวคอนเทนต์ของเรา แล้วปรับให้คอนเทนต์อยู่กลางจอ <body> จะต้องมีความสูงเท่าจอ (100% หรือ 100vh) ตัว wrapper ใช้เป็น 100vw, 100vh หรือหน่วยเปอร์เซนต์จะดีกว่าฮะ แล้วทำให้เป็น display: flex, justify-content: center, align-items: center จะทำให้ตัวคอนเทนต์อยู่กลางจอฮะ
    3. ลองเรียนเรื่อ Semantic HTML เพิ่มนิดนึง แทนการใช้ div เยอะๆ ในโปรเจคท์ฮะ เช่น <main>, <section>, <article> หรืออะไรทำนองนี้ อาจจะยังไม่ต้องลงลึกมากเรื่องการกำหนด attribute

    โดยรวมถือว่าทำดีมากๆ ฮะ พัฒนาต่อไป💪

    Marked as helpful
  • nerometa•280
    @nerometa

    Dude, this is so well done! Congratulations🎉 I legit have nothing to say since it's so good, even HTML and accessibility issues aren't present.

  • nerometa•280
    @nerometa

    You're doing great, dude! No major problems. Only minor problems that I think you can fix it.

    • Headings. It should be a bit bigger like 2rem or 36px. You also need to make sure that it's in uppercase. Can be done by text-transform: uppercase if I remember correctly

    • Buttons. Text can be bolder and font size a bit bigger.

    • Responsive: I understand that the challenge requires you to make desktop and mobile responsive, not in between. If you can make tablet responsive that's a plus.

    You can check out my solution on this challenge. I wanna hear your feedback as much as you do.

    Marked as helpful
  • nerometa•280
    @nerometa

    You could do that. What I'm gonna say here might not be the best solution though:

    Hardcode width and height of your card and use @media query to hardcode yet another width and height properties when your screen got smaller or bigger.

    You could also use min(), max() and clamp() to do sizin. It might work a bit better.

    Marked as helpful
  • nerometa•280
    @nerometa

    First of all, that was great. Your design is pretty similar to the challenge's design. I might have to checkout PixelPerfect that you using though. It's interesting.

    I think when it comes to transferring idea to code, you might have to do some UX/UI design beforehand like these challenges provide you. You can literally see how your code will look like on it.

    Maybe you cannot 'see' how the page are structured. I think learning more about different HTML tags and what and how they're use is crucial here.

    Maybe you don't know some of CSS properties. That explains you fiddling in dev tools, which I suggest you go check out CSS guides here: https://css-tricks.com/guides/ or you might have to learn the basics more.

    Marked as helpful
  • nerometa•280
    @nerometa

    I guess you're new to coding. This is good enough based on your experiences. Now, several things I want you to fix and I know you can do it:

    • Your card and image border radius. % unit is too much. Use px unit on it will look far more similar to the design.
    • You can group your 2 paragraphs together, then apply padding to it. This is more consistent and I think it's pretty responsive. If you worried about paragraph spacing, you can still add top or bottom padding to each paragraph.
    • Card height and width can use other responsive units such as % or vw for width and vh for height. Fixed units like px is hard to do responsive design.