Design comparison
Solution retrospective
Hi guys. Finally finished this project. Still have no idea how to make border of popup project green when selected. Tried every method I could find online but nothing worked. Looking forward to your suggestions.
Community feedback
- @ZhiulPosted almost 2 years ago
Hi! Your project looks really great and your code looks is mostly very clean. 😃💫 Here's some feedback:
1- You could use
<a>
elements inside your.navbar-list__element
. It could be a bit more semantic (they most likely could be links), also it would make them focusable (also, you could consider to style how certain elements should be while in focus, like buttons and so on, sometimes you will want to set a tabindex).2- Overall your code looks really great. I suggest you to use
Prettier
or some kind of tool that could format the code automatically.3- When it comes to your JavaScript code, it is great, but I think you could make it a bit cleaner by doing a few things like naming the functions, so they could be like "toggleOverlay", "toggleHamburger" and so on.
4- You are using
innerHTML
it can be ok, but usually for setting text, if you're only setting texttextContent
seems to be a better practice.5- You should assign a type for each button. In this case, I think
type="button
should suffice. Also when it comes to some elements that are button but use other element, you should considerate to use a<button>
element or at least using arole="button"
.6- You should use
rem
a bit more in order to make the page more accesible. Some parapgrahs and elements that should change stay with a static font-size. Also, you could want to set thefont-size
in your.page
in rem, so you could use 1rem. More than that, if you're using rem in some text elements, you would most likely also need to handle some paddings and properties either by using rem or em (em can be more convenient in some cases); you should consider too some edge cases like if you increase browser font-size and the viewport is small, there could be some overflow.7- When doing transitions, only transition what is necessary. In this way, you could avoid some unnecesary transitions, and overall it is a better practice.
8- You could achieve making the border green in this way:
- You use a querySelector to check if some previous project has a class of green border (or selected, though this could also be written with a data attribute just like data-selected="true").
- If some previous
.popup-project
has a class/data-attribute of green/selected remove it. - Add the class of green/selected to the
.popup-project
that has just been selected.
And when it comes to getting the parent, you could use
closest()
😃 So it could be a function that runs when you click a.popup-project__radio
.It could also be possible to use has:(), however you must take into account that right now it only has a 82% support, so you should stick to JS by now and if you decide to use it, you should at least use a fallback. https://caniuse.com/css-has
Marked as helpful0 - @codeguy9Posted almost 2 years ago
Hi. Awesome work. I see you know javascript. Can you teach me a few things?
Thanks in advance.
0
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