Skip to content
OverviewSolutions
8
Comments
12
Mark Teekman
@markteekman

All comments

  • Mark Teekman•365
    @markteekmanPosted over 2 years ago

    Hey Haico! Love your use of Marvel quotes, big fan of Marvel myself 🙂 Don't know if it's possible with the API you've used, but it would be cool to see the person behind the quote e.g. "You took everything from me." - Wanda. Just an idea though.

    To improve the accessibility and the usability of your solution a bit, consider using cursor: pointer; for when you're hovering over the randomization button.

    Great work Haico!

    Marked as helpful
    0
  • Mark Teekman•365
    @markteekmanPosted over 2 years ago

    Love how you've added that "Best of three" at the top of your solution Suleman! Really creative 😊 One tip would be to checkout the report Frontend Mentor generates for your solution, there's some really great tips on how to improve your HTML and accessibility.

    Keep up the great work!

    1
  • Mark Teekman•365
    @markteekmanPosted over 2 years ago

    I really like your solution as well Noha! Especially the way you animated how the house picks its icon 🙂 Keep up the great work!

    3
  • Mark Teekman•365
    @markteekmanPosted almost 3 years ago

    Hey Hidde,

    Nice solution! It's really close to the original design :) Be sure to also check Frontend Mentors report, it contains really helpful information about accessibility and semantic HTML! In this case, adding an empty alt="" tag to your image and wrapping your content in a <main> landmark should do the trick.

    Keep it up!

    Marked as helpful
    0
  • Duyen Nguyen•950
    @Duyen-codesSubmitted almost 3 years ago

    I hardcoded the data since I got stuck at injecting data from JSON file. Would really appreciate if you can give some tips on this problem. Thanks a lot.

    Time tracking dashboard using Grid

    4
    Mark Teekman•365
    @markteekmanPosted almost 3 years ago

    Hi Duyen,

    Great solution! Love how you combined Grid en Flex to build this one out :) Be sure to also check Frontend Mentors report, it has some great tips to improve accessibility! For example, adding an empty alt="" tag to all your images should solve all the issues in this case.

    You could also improve the design a little by making 'daily', 'weekly' and 'monthly' into anchor links with an active and hover state :)

    As for the data, I'll reference it here again so others might read it too: the JavaScript Fetch API can be very useful in this case. You'll need some JS to interact with the data, there are plenty of ways to do it, this is how I solved it.

    Keep up the great work!

    Marked as helpful
    0
  • Mark Teekman•365
    @markteekmanPosted almost 3 years ago

    Hi Nick,

    Great first solution! Haico has already made some nice additions. Things I could think of (I took some of these from my comment at Haico's solution):

    • When viewing the component on a small screen size the card overflows the viewport. This is because you've set a fixed width: 302px on the #card element. Try to avoid setting it explicitly as much as possible. Setting max-width: 302px solves this.
    • Consider using classes instead of id's to style your HTML. It's lower in CSS specificity and a better practice for re-usability.
    • Your anchor links aren't distinguished in this component. 'Equilibrium #3429' and 'Jules Wyvern' are links, but this is not visually notable. Consider adding an underline to them for accessibility purposes.
    • Your attribution links have a color contrast ratio of 1.88. The minimum contrast ratio is 4.51, a nice one would be a ratio of 6.0. You could achieve this by using a white color and an underline for example.
    • You can add cursor: pointer to your .overlay:hover class to add that extra level of interaction taken from the design :)

    Keep up the great work and happy coding!

    0
  • Mark Teekman•365
    @markteekmanPosted almost 3 years ago

    Haico, just figured I forgot one addition, you can add cursor: pointer to your .header:hover .view-container class to add that extra level of interaction taken from the design :)

    1
  • Mark Teekman•365
    @markteekmanPosted almost 3 years ago

    Hey Haico,

    Great work once more on this challenge! You can definitely see your CSS Grid skills improve :) I like how you kept the HTML very minimal and used the power of Grid to handle the structure of the layout.

    Some small improvements:

    • On smaller screens (phone for example) the attribution footer lays over your card because you've used position: fixed. You could try something like a sticky footer solution, where it's still at the bottom on large screen sizes but stays below the main content on small screen sizes.
    • When viewing the component on a small screen size the card overflows the viewport. This is because you've set a fixed width: 375px on the main element. Try to avoid setting it explicitly as much as possible. Setting max-width: 375px solves this.
    • Try to include a CSS reset in your project, not including one can often lead to differences between browsers. A good example is trying to implement the sticky footer I mentioned. You can see in Firefox that the body has some margin which results in the footer overflowing the viewport because of the width: 100vw the footer has. There are many options, a simple one is all you need.
    • When using alt="" attributes on images, check whether is makes sense to put it there. For example, you have alt="image of Jules Wyvern" on the avatar, followed by the text 'Creation of Jules Wyvern'. A screen reader would now read 'image of Jules Wyvern Creation of Jules Wyvern'. So here, it's better to have an empty alt="" tag. If the avatar image wasn't accompanied by any text it would make sense to set the alt tag to describe it to the user.
    • Another accessibility concern is that your anchor links aren't distinguished in this component. 'Equilibrium #3429' and 'Jules Wyvern' are links, but this is not visually notable. Consider adding an underline to them.

    All in all, it's a great looking component!

    Happy coding :)

    Marked as helpful
    2
  • Haico Paulussen•170
    @Haico-PaulussenSubmitted almost 3 years ago

    I'm very proud of my work. But I want to learn how I can do things better. Especially using CSS Grid. So if you have any comments or improvements I really want to know so I can keep on learning and getting better!

    Mobile first, CSS Grid

    1
    Mark Teekman•365
    @markteekmanPosted almost 3 years ago

    Hey Haico,

    Great solution using Grid! I like how you’ve implemented a couple of the shorthand’s and things like min-content, nice use case!

    Also, don’t forget to check Frontend Mentor’s Accessibility Report, it’s a good way to learn about some fundamental HTML stuff.

    Just so it happens, Kevin Powel (the CSS Wizard) did a YouTube video on CSS Grid using this exact same Frontend Mentor challenge as an example, would be a nice way to compare different approaches: https://www.youtube.com/watch?v=rg7Fvvl3taU.

    Keep up the great work!

    Cheers, Mark

    Marked as helpful
    1
  • Mark Teekman•365
    @markteekmanPosted about 3 years ago

    Very nice solution you made here! I really love seeing you've used Astro :) I've got a couple of pointers to further improve your code and the user experience of your solution:

    • Your menu items are all <h1>'s, for accessibility it's important to only use one <h1> per page. You could instead just put your menu items in the <a> tag alone and apply the necessary styling there
    • When hovering over the menu items they become bold and this creates a shift in layout. You can solve this by setting your menu items inside unordered list items and apply a min-width on them to prevent this shifting. I did the same for the menu in this solution: https://markteekman.github.io/easybank-landing-page/
    • Consider adding cursor: pointer to your light and dark mode toggles to make it more clear to the user that it's interactive
    • On a single product page you can apply a gap: 2rem property on the parent div that wraps the little preview images to give them some space like the original design
    • Consider putting some padding on the container for smaller viewports on the single product page, that way there will be some breathing room between the big image and the browsers edge
    • Checkout Frontend Mentors report on your solution, it's a great way to learn more about accessibility and correct HTML mark-up

    Hope these help, keep up the great work and keep it up with Astro! :D

    Happy coding! Mark

    Marked as helpful
    1
  • Mark Teekman•365
    @markteekmanPosted about 3 years ago

    Hey Haico, nice first solution, looks great 👍🏼

    Some suggestions on how to improve your code:

    • You could try using a <footer> landmark instead of a div with a class of footer for better markup
    • Also wrap your card in a <main> tag for a better and more accessible markup (these first two points should tackle most of your accessibility warnings)
    • You could set the background on the <body> instead of using a separate div to clean up the code a bit

    Keep up the good work!

    Marked as helpful
    1
  • MikevPeeren•2,100
    @MikevPeerenSubmitted about 3 years ago

    Hello all,

    I would mainly want to know if the styling could be improved. Any other feedback is also welcome of course :D

    All issue's displayed on Frontend Mentor are related to Next.js, if there are also tips on how to remove them please advise.

    Time Tracking Dashboard with Next.js/React/TailwindCSS

    2
    Mark Teekman•365
    @markteekmanPosted about 3 years ago

    Nice one Mike!

    Looks really great and some great suggestions by Dharmik48 already :) Some additions:

    • You can also improve the design a little more by setting the ProfileCard width to 20% to make it resemble the original design a little more.
    • There's an error in the console about an uncaught promise, but maybe that's also a Next.js thing because switching the stats does work.

    Keep up the good work!

    Marked as helpful
    2