Skip to content
OverviewSolutions
7
Comments
3
Volkan Guneri
@volkanguneri

All comments

  • Volkan Guneri•170
    @volkanguneriPosted almost 2 years ago

    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
  • Volkan Guneri•170
    @volkanguneriPosted almost 2 years ago

    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
  • Volkan Guneri•170
    @volkanguneriPosted almost 2 years ago

    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