Design comparison
SolutionDesign
Solution retrospective
My first project utilizing JavasCript. If you have any suggestions, please comment ^^
Community feedback
- @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✅:
- Add aria labels , not only helping your code quality better but also make your website more accessible.
- Make your font families be added in locally to your project file as it make it look better.
- 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 helpful2 - @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:
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 thebody:has(#switch:checked) > #value1-checked { display: initial; } body:has(#switch:checked) > #value1-unchecked { display: initial; }
display
property. Another approach to this could be by using thecontent
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, theborder-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
andactive
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 helpful1 - This could be done in the following way:
Please log in to post a comment
Log in with GitHubJoin 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