@Haico-Paulussen
Submitted
Looking to hire developers?
@markteekman
@Haico-Paulussen
Submitted
@markteekman
Posted
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
@legion40216
Submitted
Any improvement Feedback
@markteekman
Posted
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!
@NohaaAa
Submitted
@markteekman
Posted
I really like your solution as well Noha! Especially the way you animated how the house picks its icon 🙂 Keep up the great work!
@hiddehulshof
Submitted
@markteekman
Posted
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
@Duyen-codes
Submitted
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.
@markteekman
Posted
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
@nicktelindert
Submitted
@markteekman
Posted
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):
width: 302px
on the #card
element. Try to avoid setting it explicitly as much as possible. Setting max-width: 302px
solves this.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!
@Haico-Paulussen
Submitted
@markteekman
Posted
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 :)
@Haico-Paulussen
Submitted
@markteekman
Posted
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:
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.width: 375px
on the main
element. Try to avoid setting it explicitly as much as possible. Setting max-width: 375px
solves this.width: 100vw
the footer has. There are many options, a simple one is all you need.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.All in all, it's a great looking component!
Happy coding :)
Marked as helpful
@Haico-Paulussen
Submitted
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!
@markteekman
Posted
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
@Artmade-studio
Submitted
@markteekman
Posted
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:
<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 theremin-width
on them to prevent this shifting. I did the same for the menu in this solution: https://markteekman.github.io/easybank-landing-page/cursor: pointer
to your light and dark mode toggles to make it more clear to the user that it's interactivegap: 2rem
property on the parent div that wraps the little preview images to give them some space like the original designHope these help, keep up the great work and keep it up with Astro! :D
Happy coding! Mark
Marked as helpful
@Haico-Paulussen
Submitted
@markteekman
Posted
Hey Haico, nice first solution, looks great 👍🏼
Some suggestions on how to improve your code:
<footer>
landmark instead of a div with a class of footer for better markup<main>
tag for a better and more accessible markup (these first two points should tackle most of your accessibility warnings)<body>
instead of using a separate div to clean up the code a bitKeep up the good work!
Marked as helpful
@MikevPeeren
Submitted
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.
@markteekman
Posted
Nice one Mike!
Looks really great and some great suggestions by Dharmik48 already :) Some additions:
Keep up the good work!
Marked as helpful