I did the bulk of this a month ago, and then got sidetracked. At the time all that was left to do was the spacing between sections, and inside the table. Also how to handle the header image using full width for mobile, and inside the card for desktop. (I think this was why I stopped at the time)
I'm aware of the H3 warning above. Will correct in due course.
Great job, can't / won't comment on Tailwind / next.js build as I don't know enough.
Its nice and responsive, breaks nicely between the two sizes.
If you wanted me to be really (😹) pedantic there are two things in the "Preparation Time" callout:
First the wrap of "minutes" on mobile isn't in line with Total. However there is alignment further down the page on others that wrap.
Second, in the callout the : is outside the span, but further down you have the : inside the span. To me both should be inside the span.
Looks great, and its responsive to resizing. Can't really comment on the code because I know nout about React/Tailwind so far.
Respondeds to tabbing which is good.
Only suggestion is breaking at 768px looks rough, you'd be better being two columns down closer towards the mobile size. Maybe 500? If I was looking on my iPad I'd feel it was a huge waste of screen real estate.
Have SVG's for each of the social services but didn't end up adding them as an "enhancement"
Hey congrats on finishing the challenge. There are some issues, particularly in the between sizes.
aria-labels to the social links so that a screenreader will read out what the links are for. Particularly regarding the social links..offer-price-box has a z-index of 0 after 800px. You set z-index: 3 in the media query upto max-width: 50rem.:hover, also add :focus, you could do .ios-btn:where(:focus, :hover) that way someone using the keyboard to navigate around the page gets the effect too.<br> after the year in the footer.alts for images (on patterns, logos). Add a description, and it you feel its not an important thing for not sighted people, then use aria-hidden on them.apple class for both the apple and android svgs.Hope this was of some help to you.
Hey congrats on completing the challenge. Looks great, and nicely responds to screen being shrunk. (Can't comment on the code as I don't know tailwind)
Couple of little things:
a tags, which are selectable via the keyboard. Not li tags.:focus as well as :hoverdiv for everything. Try and use the approriate semantic tags where you can. For instance the bottom section should be footer.Congrats on completing the challenge, and making it nicely responsive. I also liked that you catalogued some of your issues in your Readme. That was good.
I thought the use of flexwise, flowise and paddwise was interesting. Something I'll look at for my own solutions.
Hi Alex 👋 congrats on finishing the challenge. I'm impressed I've not see the HTML validator throw out so many errors before. Seems like you broke it somewhere but not with your code (give one of the errors mentions webkit but I couldn't see that in your stuff).
Your build looks good on the expected sizes, but you're right that some of the sizing look broken. For instance 470 to 770 ish, and 1024 to 1440. Part of that is you are adding explicit width & heights in your media queries
@media (min-width: 768px) and (max-width: 1024px) {
body {
background-image: url('../bg-main-tablet.png');
width: 768px;
height: 1781px;
}
//// snip
}
@media (min-width: 1025px) and (max-width: 1280px) {
body {
background-image: url('../bg-main-desktop.png');
width: 1440px;
height: 1832px;
}
}
@media (min-width: 1281px) {
body {
background-image: url('../bg-main-desktop.png');
width: 1440px;
height: 1832px;
}
}
Don't do that. For a start makes the design very unresponsive and adds unneeded horizontal scrolling.
Add a <br> tag after "All rights reserved © Equalizer 2021" to force the rest onto the next line. Maybe make the email bold, or a mailto: link.
Keyboard navigation between your buttons doesn't work.
aria-labelledby="footer-heading absolute bottom-0" doesn't look right. This article describes its usage
Wishing you the best for your coming challenges.
Hi @Lisviks just wanted to say congrats on the challenge solution, I think it looks great across all three sizes. Also yours is the first solution I've commented on so far that correctly put the <br> after the All rights reserved © Equalizer 2021 🤪
<picture>
<source media="(min-width: 1440px)" srcset="./assets/bg-main-desktop.png" />
<source media="(min-width: 768px)" srcset="./assets/bg-main-tablet.png" />
<source media="(min-width: 375px)" srcset="./assets/bg-main-mobile.png" />
<img src="./assets/bg-main-desktop.png" alt="background image" class="bg-image" />
</picture>
Was a good way of handling the bg images.
For the Footer, I'd recommend against doing that social links the way you have. They should be links (even if they aren't going to real pages), with some sort of aria-label to indicate where they are going.
I'd recommend using aria-hidden on the various images / product logos.
Don't put the hover for the socials only on the Desktop media query. You never know if the user might just have their browser window at tablet size because they are using a different app beside it.
footer .socials div:hover {
cursor: pointer;
background: #fa7453;
}
Hope this was of some help. Wishing you the all the best.
Congrats on another tweaked solution Lucas 🎊
Couple of little things, I'd suggest breaking at 1280px for Desktop so the "sic" of music doesn't overlap the top right image. Particularly because the cycling colour matches that image.
The <br> should be after "All rights reserved © Equalizer 2021"
Mixed feelings about the changing the colours on the phone image. Nice touch but I find it distracting. It becomes a "wait what did I miss", and the pink looks between cyan and orange looks odd. Similar with the equalizer logo at the page top.
The product box does some odd shifts between about 730px & 450px. It compresses and expands, compresses and expands. It gets really thin about 490px. Honesly having so many media queries makes is working against you and gives you more to check/test. (Not that I can comment, I did my middle section incorrectly, so my responsiveness between tablet and desktop sizes is awful. Its always easier to critique others 🤪 )
Would the aria label aria-label="equalizer app pricing" not be before on the div just before the $4?Particularly given the h & p before the price are to do with pricing.
For the social links / images. Should the links not have an aria label on them describing what they are? And the icons not have aria-hidden=true?
This was interesting, not come across text-decoration-skip-ink before.
/* A elements that don't have a class get default styles */
a:not([class]) {
text-decoration-skip-ink: auto;
}
Given you are using transitions, you should provide a version for those that have prefers-reduced-motion set.
Wishing you all the best 👨💻💜
Hi Matt, congrats on your solution. First I'm impressed you built out the design system as an html page. Looks identical to my memory of the figma file. Might help some out who want to take a stab at the challenge themselves.
I like that you added some .sr-only bits for the social icons, although I don't think the brackets are needed :)
Interesting seeing font sizes categorised by their font weight. I think I've seen Kevin Powell doing that too.
Good to have both a btn class and then specific iOS and Android buttons.
I would suggest if you are going to do multiple media queries as you have, to use a variable for the width so you don't have to repeat the numbers. Could lead to a subtle difference if you use 43 for most, then one thats 42 or 44.
Looking at responsive design mode I'd suggest breaking at 1200px so the fullstop isn't over the top right image. (Rather than 1153px) That said, your build is MUCH more responsive than mine. I built the middle area the wrong way around. So looks pretty poor from Tablet to Desktop sizes.
Anyway congrats, and all the best 👨💻
Hi congrats on finishing the challenge. Please when you submit it, link directly to the repo / folder with the code and not your Github profile. Its not clear looking at your profile that the repo contains your solution. For anyone coming along, solution is https://github.com/lumon2004/lumon2004/tree/Frontend-Mentor-Challenges/NFT%20preview%20card%20component
I'm curious why you chose to use the Cabin font, rather the one indicated in the Style guide? Likewise the change of page background color.
If you have a good reason that would be something perfect to add to the Readme.md (rename existing Readme.md to something else, then rename Readme-template.md to Readme.md. Then edit it to cover what youve done)
I'd suggest not mixing styling between a style block and the html tags themselves. It makes someone reading your code have to jump around to understand what is going on.
Try and use semantic html where approriate. For instance wrap the attribution in a footer tag. The container could be a main tag.
Your alt tags are fairly descriptive which is good, although "NTF" is a touch vague.
I don't think using h2-5 is the right approach for the text in the card as they aren't really headings. For instance the h3 for price and h5 for term/period of time.
For the image, "Equilibrium #3429" and creator name you are styling them as if they are links. So make them links.
Test at in between screen sizes using the responsive design mode in a browser. For instance your build is fine at 375px, but breaks horribly at about 425px until 1138px. This happens because your container is set to width: 20%; above 425px.
Try and take a mobile first approach, and then adapt as needed for the desktop version. For instance use width: 80% as you have for mobile, but then have say max-width: 21.875rem on the container so it never gets bigger than that size. Then your solution will scale nicely from mobile to desktop (you'll need to tweak padding as well)
Hope that helps, feel free to ask if I've been unclear, and best wishes for your next challenge.
Oh fudge, my comment failed, apologies this will be more terse. Congrats on finishing the challenge, it looks good. I don't see the problem with the preview image.
I can't really comment on the CSS/Tailwind, I haven't used it and don't like how cluttered it makes the HTML look. I find it hard to grok.
First I know these are just challenges, but get into the habit of providing useful alt text. Don't leave it as simply alt="" as you have here.
Where you can use semantic html rather than divs. For instance the first div has a role of main, in which case, why not use the main tag?
If an icon isn't important for screenreaders and such then add aria-hidden=true to them. Consider aria-label to provide more info that a sighted person might not need. For instance have an label that said "The price of the NTF image is 0.041 ethereum"
Best wishes for your next challenge.
Hi congrats on finishing the challenge 🎊. You've defintely got the spirt of the challenge.
Try and be more descriptive with your alt descriptions, for instance if a screenreader read out "equilibrium" it isn't going to tell the user anything about what the image is.
For unimportant icons, like the ethereum logo or the clock, use something like aria-hidden=true and add aria-label if you feel that more description might be needed. For example "This image costs 0.041 ethereum", for the period you might add something as well.
Also you have no alt for the overlay image. Either set it to aria-hidden, or add a label describing what is happening.
In theory the image preview and the title are also links, like the creator name.
I wouldn't recommend setting the font-size in the HTML selector. As then all of your use of rem are based off of 18px. Someone else reading your code may miss that and wonder why values aren't what they expect / you end up with odd fractions.
If you have access to a measuring tool, check out the width of the card for both the mobile and desktop versions. Both are larger than 300px, but also different than each other.
Best wishes for your next challenges.
Hi Nutchapon, congratulations on completing the challenge 🎊 Looks good, and works for both desktop and mobile. As I don't really know react I can't comment on the build of your solution.
Be careful with the CSS, for instance the root doesn't need the max-width: 1440. The only thing that needs a max-width would be the card itself.
Also for your link hover's you used color: hsl(178, 100%, 50%) !important;. Don't use !important if at all possible. Its a last resort. As it is, your hover solution for a tags works without it.
The text-rendering: optimizeLegibility; is new to me, so cheers for that. However as far as I'm aware font-weight: 300, 400, 600; doesn't mean anything. The weights are imported when you import the font. You then just use the font-weight you need for a particular block of text (like the heading being 600) https://developer.mozilla.org/en-US/docs/Web/CSS/font-weight
Really nice solution.
One comment I have is that while the copy icons prevent you from copying the placeholder text, you can just drag & select the placeholder text and copy it.
The toast is a nice touch, although personally I'd prefer it at the top of the screen (desktop) near the copy icon because that is where my eyes are focused.
(Complete aside, this could be safari, there is an odd glitch where sometimes the whole bottom box area jumps when you move the slider from 0 to 20, happens around 9 or 10. Usually happens when you move it quickly but sometimes slowly)
Well done on completing the challenge, unfortunately it looks like the deploy to a site hasn't worked.
Glancing at the CSS, my main comment would be to approach the design mobile first. That way you aren't duplicating a bunch of the CSS for mobile and desktop versions.
Also a little confusing that in the desktop version you just use h1 and p to style the .info section, but in the mobile version you use .info h1 and .info p, and section .price in desktop and .plan .price in the mobile version.
Aim to be consistent, otherwise it could be confusing for someone else who has to maintain your code.
Best wishes for your next challenge.
Congrats on finishing the challenge. Some thoughts/notes:
--primary-color1 doesn't tell me muchfont-family: 'Lexend Deca', sans-serif; in the appropriate place. For instance either body or container. Then specify Big Shoulders for your h1H1 per page. So you would be better using h2's here