Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found

Submitted

Pricing Component with toggle

@CrystalNyein

Desktop design screenshot for the Pricing component with toggle coding challenge

This is a solution for...

  • HTML
  • CSS
  • JS
2junior
View challenge

Design comparison


SolutionDesign

Solution retrospective


Feedbacks are appreciated.

Community feedback

@pikapikamart

Posted

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 the overflow-x on it. If you inspect the layout in dev tools at the bottom, the layout creates another vertical scrollbar because if you set a overflow-x it will default the overflow-y to scroll instead of visible.
  • Those 2 background-waves could just be used as background-image so that you won't have to create extra img tag, but if you insist on doing so, add aria-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 the input which you NEVER should do. Doing so removes the interactive element from the dom itself. On this one, if you think about it, this is a selection of choice right, therefore using radio-buttons input type="radio" is much preferable. For this one, you should use a fieldset to wrap the toggle, a legend tag which its text content should describe what is the purpose of those input. Use 2 label and two input type="radio". Those text, annually and monthly are label. 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 the div container into using fieldset and add maybe an sr-only legend tag inside it.
  • Each plan's pricing, like 199$, they could just a p tag since the h2 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 using a tag rather than button 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

0

@CrystalNyein

Posted

@pikapikamart Wow that's great help. I know that my coding styles are imperfect and got no one to point out which parts I need to improve. With your comment, I know how I can improve on my project. Next, the dos and don'ts are really helpful.

Thank you so much.

1
Carl Wicker 1,055

@carlwicker

Posted

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

0

@CrystalNyein

Posted

@carlwicker Thank you Carl. I'll try to fix that part.

0

Please log in to post a comment

Log in with GitHub
Discord logo

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