Pricing Component

Community feedback

• 

  Watership 🌊•320

  @Watership6Posted about 1 year ago

  Nice job😁,

  I really liked your solution especially for being the first one you have done with JS. I have a few critiques that I would like to suggest you add to make your code better✅:

  1. Add aria labels , not only helping your code quality better but also make your website more accessible.
  2. Make your font families be added in locally to your project file as it make it look better.
  3. A very small change but make the circle in your toggle a bit smaller to make your website look a bit better. ⭐

  ---

  Overall, I really liked your solution and can't wait for your next solution. Thank you for sharing you solution with us. 🧑‍💻

  Sincerely, Watership🌊💧🛩️

  Marked as helpful

  2
• 

  Kaustubh Maladkar•230

  @KaustubhMaladkarPosted about 1 year ago

  Hi Weldor, Congrats on utilizing JavaScript in your first project. However, this project did not need JavaScript and could be completed using only HTML and CSS.

  • This could be done in the following way:

    body:has(#switch:checked) > #value1-checked {
      display: initial;
    }
    body:has(#switch:checked) > #value1-unchecked {
      display: initial;
    }
    

    In the example provided above, you make separate elements for the two scenarios (checked switch, and unchecked switch). Based on whether the scenarios are true or not, you display the required elements, and hide the others using the display property. Another approach to this could be by using the content property, but this would make the text inaccessible. Using JavaScript makes the page load slower, and is also bad for responsiveness. That said, the :has() pseudo-class does not have widespread browser support. I would suggest that you use SCSS to achieve the aforementioned, because of the amount of nesting involved.
  • Your class and ID names are meaningless. Using names like "first" and "second" is not a best practice as the names don't do a great job of explaining the content present in these containers. For example, you have given your first div the ID of "second1". Instead of this, an ID like "basic" would be much better
  • Instead of using the 'div' tag as a container, I would use the 'section' tag, which is used the content relates to a single theme.
  • The <hr> tag is deprecated. It should never be used. To obtain the styling you wanted, the border-bottom property of CSS was a much a better option
  • Several of your buttons are not accessible through keyboards. Provide them with styles for the focus and active state.
  • The value of you "learn more" buttons is in caps. Why? For styling purposes. For any styling purposes, whatsoever, it is always better to use CSS. If I were in your shoes, I would utilize the following CSS property: text-transform: capitalize.

  Marked as helpful

  1