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

Practice-9

@SvitlanaSuslenkova

Desktop design screenshot for the Interactive rating component coding challenge

This is a solution for...

  • HTML
  • CSS
  • JS
1newbie
View challenge

Design comparison


SolutionDesign

Solution retrospective


Will be glad to receive any advice on my js code.

Community feedback

kimchiver 90

@kimchiver

Posted

Hello!

Your coding of design is excellent. (I like your History Back style!)

I'd like to add a few js issues to the above:

-- "blur" event does not work properly with button elements, at least in Safari /Version 16.6/. I suggest to do inactive-button.classList.remove("selected") for all buttons inside a "click" callback-function of the active element (like @RalfiSlask noticed)

-- it's better to use add/remove(class) instead of toggle(). When the user clicks the same number twice, toggle() switches off the active mode.

It may seems overwhelming today, but don't give up and keep up! In case you get into this, I'd like to recommend a really good source for web-development learning - www.w3schools.com

Marked as helpful

0

@SvitlanaSuslenkova

Posted

@kimchiver Thank you! It helped me. It works better!

0

@kabir-afk

Posted

Hey , I think @RalfiSlask rectified your errors regarding JS , I saw some problems with CSS and thought I should help . . .

  • In your submit button as well .rate you should set font-family:inherit so that it goes along with the rest of the page , buttons and other input elements do not generally inherit it directly but rather have to be specified otherwise they display default font i.e., Arial

  • NOT using commas to add styles to multiple elements/hover states . . . like this might function properly but shows error in editor so yea . . . .

.submit:visited{}
.submit:hover {background-color: hsl(0, 0%, 100%); color: hsl(25, 97%, 53%);}
.submit:active {}

The code should have been

.submit:link ,
.submit:visited,
.submit:active ,
.submit:hover {background-color: hsl(0, 0%, 100%); color: hsl(25, 97%, 53%);}

The site is ok otherwise . . .Hope I was able to help, happy coding!!! 🥂🥂

Marked as helpful

0

@SvitlanaSuslenkova

Posted

@kabir-afk Thank you very much! Your advice is useful too.

0
P

@RalfiSlask

Posted

Hello!

Javascript feedback

Variable Names

Change the name of btn to buttons as it targets all the buttons and not a single one, dont be afraid of writing larger names as it is important for other programmers to be able to understand your names. For example bck can be really hard to understand what it means.

Selectors

Have all the selectors at the top of your file.

Using var instead of let and const

I would suggest using the more common standard let and const for declaring variables as var is a bit outdated. It is generally better to use let and const instead of var because of their nature in scoping and hoisting which can lead to some bugs if you are not familiar with its nature. Also how you declare your functions changes the hoisting, arrow functions will not be hoisted where function() will be.

Let and const are block scoped and var is function scoped, var will also be hoisted which means that it bubbles up to the top of the file. Block scoped means that you can only handle the variables inside blocks for example if else where function scopes means that you can reach variables everywhere inside a function.

const checkScope = () => {
  console.log(varInsideBlock) // will print undefined 
  cause of hoisting this becomes the declaration var varInside; 

  if(1 === 1) {
    var varInsideBlock = "function scoped";
    let letOrConstInsideBlock = "block scoped";
  };

  console.log(varInsideBlock) // will print 
  console.log(letOrConstInsideBlock ) // will produce ReferenceError
};

checkScope();

Array methods

Use array methods instead of for loops when you are working with arrays or NodeLists. For loops are better if you are working with numbers etc.

buttons.forEach(button => {
  button.addEventListener("click", function() {
    button.classList.toggle("selected");
    result = button.innerHTML;
    span.innerHTML = result;
  })
  button.addEventListener("blur", function() {
    button.classList.remove("selected")
  })
});`

Marked as helpful

0

@SvitlanaSuslenkova

Posted

@RalfiSlask Wow, thank you very much!!! I'll try to change it.

0

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