vineet kumar• 300
@djbravo12
Submitted
Volkan Guneri
@volkanguneri
All comments
- Volkan Guneri• 170
@volkanguneri
Posted
Good job! May some suggestions can improve your code.
Using classes for css and no id's whiche are for javascript ineractions. Adding a cursor:ponter; for your button and delete all comments that are not usefull. Organising js code more clearly //variables //functions //eventlisteners
Cheers
0 - Jason Solarz• 60
@JasonSolarz
Submitted
Volkan Guneri• 170@volkanguneri
Posted
Awesome!
I can suggest you in CSS
1-
background-image: url('images/icon-star.svg') center center / cover no repeat;
instead of
background-image: url(images/icon-star.svg); background-repeat: no-repeat; background-position: center;
2-
use of rem and em instead of pixels
3-
u can also use css :active proporty instead of javascript here by adding tabindex="1" to your "li" in HTML.
li:active { color: hsl(0, 0%, 100%); background-color: hsl(217, 12%, 63%); }
instead of creating a new css class
.selected { color: hsl(0, 0%, 100%); background-color: hsl(217, 12%, 63%); }
4- you can also manipulate css directly by javascript DOM instead of creating a new css class like
.display-not { display: none; }
main.style.display = 'none'; section.style.display = 'block';
I hope that it can make more economy.
I also learned from your javascript code.
Thanks
0 - Rajab Ali• 20
@Alijutt75
Submitted
Volkan Guneri• 170@volkanguneri
Posted
Great Job! I only noticed that you have a hover and active problem on your numbers. Hover is orange indeed but active color is light-grey. And when we go on "3" for exemple, "1" "2" and "3" become all orange intead of only "3" if i understood.
Marked as helpful
0
