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

Welder Victorโ€ข 80

@weldersalvador

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


My first project utilizing JavasCript. If you have any suggestions, please comment ^^

Community feedback

@Watership6

Posted

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

@KaustubhMaladkar

Posted

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

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