Pricing Component with toggle

Community feedback

• 

  Raymart Pamplona•16,100

  @pikapikamartPosted almost 3 years ago

  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

  ---

  

  Nyein Chan Lay•350

  @CrystalNyeinPosted almost 3 years ago

  @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

  @carlwickerPosted almost 3 years ago

  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

  ---

  

  Nyein Chan Lay•350

  @CrystalNyeinPosted almost 3 years ago

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

  0