Pricing Component with toggle

Solution retrospective
Feedbacks are appreciated.
Please log in to post a comment
Log in with GitHubCommunity feedback
- @pikapikamart
Hey, awesome work on this one. Both desktop and mobile state actually looks really great.
Carl Wicker already gave a feedback on this one, just going to add some suggestions as well:
- It would be nice to add the
box-sizing: border-box
on the*
selector so that content sizing will be much easier to handle. - On your
main
tag, remove theoverflow-x
on it. If you inspect the layout in dev tools at the bottom, the layout creates another vertical scrollbar because if you set aoverflow-x
it will default theoverflow-y
to scroll instead ofvisible
. - Those 2 background-waves could just be used as
background-image
so that you won't have to create extraimg
tag, but if you insist on doing so, addaria-hidden="true"
on both so that they will be totally ignored by screen-reader. - For your pricing toggle, it works but only limited for mouse click because you used
display: none
on theinput
which you NEVER should do. Doing so removes the interactive element from thedom
itself. On this one, if you think about it, this is a selection of choice right, therefore using radio-buttonsinput type="radio"
is much preferable. For this one, you should use afieldset
to wrap the toggle, alegend
tag which its text content should describe what is the purpose of thoseinput
. Use 2label
and twoinput type="radio"
. Those text,annually
andmonthly
arelabel
. Have a look at this old solution of mine in this challenge. But :>>, this is an old solution where I still don't know some semantic markup. But take a look at the toggle, just replace thediv
container into usingfieldset
and add maybe ansr-only
legend
tag inside it. - Each plan's pricing, like 199$, they could just a
p
tag since theh2
above them already gives information on what each section would contain. Because those numbers alone when used as heading doesn't really give that much content. - Those 3 information below each pricing could use a
ul
tag since those are "list" of information about the plan. - The
learn more
is better usinga
tag rather thanbutton
because if this were a real site, it would much likely be page link where user can "learn" more about a specific plan.
Aside from those, great job again on this one.
Marked as helpful - It would be nice to add the
- @carlwicker
Hey ya Nyein, Great job, I cant see any issues. There is a weird media breakpoint on the desktop version which makes stuff jump around. Personally I'd lose the additional break point.
Keep up the great work.
Marked as helpful
Join our Discord community
Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!
Join our Discord