Skip to content
OverviewSolutions
10
Comments
7

Giang Nhật Khánh (Kayden)

@KaydenGiang2512Vietnam150 points

I'm an IT student at RMIT University trying to master the way of the code (and hopefully land a job in the future) :D

Latest solutions

View more solutions

Latest comments

  • Giang Nhật Khánh (Kayden)•150
    @KaydenGiang2512

    Hi Muneeb, it seems that your project is incomplete, as the QR image is missing from the page and only the text section is visible. Please check your code and fix any errors you may have made. Good luck!

  • Giang Nhật Khánh (Kayden)•150
    @KaydenGiang2512

    Brilliant work Kent! I just noticed a single accessibility issue in the report generated by Frontend Mentor, where there is no <h1> tag in your entire document. Other than that, your solution was very close in resemblance to the original design. One suggestion that I would make is to set up all your colors as custom properties to be used widely across your stylesheet simply by calling var(--your-color).

  • Giang Nhật Khánh (Kayden)•150
    @KaydenGiang2512

    It was probably because I didn't phrase my suggestion well enough (quite hard to do so through text-only comments!), so I will paste in the link to my solution here so you can see what I tried to explain above. Hope this clears up any confusion you may have on our issue!

    https://github.com/KaydenGiang2512/Article_Preview_Component

    Best of luck :D

    Marked as helpful
  • Giang Nhật Khánh (Kayden)•150
    @KaydenGiang2512

    Hi Lacey, great work on this project overall. I actually completed this project a few weeks ago so I might have a solution to your "eye image opacity" issue, which is by separating the eye-icon from the NFT image into 2 separate <div> tags. After that, create another <div> as the overlay and use position: absolute for all of these to ensure that they stack on top of one another. By putting the eye-icon in its own <div> tag, the opacity attribute from the NFT image will no longer affect it, and you can independently set the opacity of the eye to your choosing. Hope this helps and happy coding!

    P.S. the image is not horizontally centered in the entire card element, so that's another thing you should look into.

  • Giang Nhật Khánh (Kayden)•150
    @KaydenGiang2512

    The result seems pretty nice for your first attempt, and responsiveness is good! I just have some feedback based on my personal opinion:

    1. The card seems to small for desktop (in comparison to the original design) so the content within is cramped and therefore some elements aren't exactly fitted well inside, for example the rating circles and the star icon on the first card.
    2. The font color on the submit button should be white (for the sake of the provided design)
    3. font sizes are smaller than the required ones (15px) which could largely be due to the limited height and width you set for the containers (refer to my first feedback). Frontend Mentor always provides design screenshots as well as parameters for most of the elements on the page, so keep an eye on those in your future projects to replicate the original as well as you can.

    Other than these comments, you did an excellent job overall and best of luck to you on your next challenge!

  • Giang Nhật Khánh (Kayden)•150
    @KaydenGiang2512

    Design-wise, this is a solid project for a beginner with everything looking nice and centered. However, the link to your GitHub repository isn't working Alexandra, please double-check in case it might be incorrect!

    For the code, I noticed 2 huge selector chunks you made on the css (the ones with lots of tags): html, body, div, span, applet, object, iframe, h1, h2, h3, h4, h5, h6, p, blockquote, pre, a, abbr, acronym, address, big, cite, code, del, dfn, em, img, ins, kbd, q, s, samp, small, strike, strong, sub, sup, tt, var, b, u, i, center, dl, dt, dd, ol, ul, li, fieldset, form, label, legend, table, caption, tbody, tfoot, thead, tr, th, td, article, aside, canvas, details, embed, figure, figcaption, footer, header, hgroup, menu, nav, output, ruby, section, summary, time, mark, audio, video { margin: 0; padding: 0; border: 0; font-size: 100%; font: inherit; vertical-align: baseline; }

    /* HTML5 display-role reset for older browsers */ article, aside, details, figcaption, figure, footer, header, hgroup, menu, nav, section { display: block; }

    which were quite confusing to me as to what you're trying to achieve because CSS can specifically target elements in your html by selecting them individually, instead of gathering them all at once. This is important for readability as well as maintaining your code going forward.

View more comments