Interactive Rating Component using SASS, CSS Flexbox and JavaScript

Community feedback

• 

  Elaine•11,360

  @elaineleung

  Posted almost 3 years ago

  Hi Yaika Race, great job in completing this first JS challenge! Everything works well, and I like how you used some logic to handle what happens when nothing is selected. Great work also in displaying a warning reminder!

  Two suggestions I have here:

  1. Instead of using <p>, use an interactive element like <button> instead, as p is really not meant for this task (it's more of a pure text element), and buttons are more suited since they handle actions. Buttons also allow for attributes such as value, and so you can store a value that you can later retrieve in the JS (e.g., value="1").

  2. Great job using the forEach! I see that you also have the idx in your function and that you later use the index for displaying the score. A simpler way that does not involve adding 1 to the index would be to just retrieve the textContent, or if you do use a value attribute in the button as I mentioned above, then you can use event.target.value or even just the element's value. To see this in action, you can check out this mini CodePen I made for this challenge: https://codepen.io/elaineleung/pen/RwMqMxZ

  Anyway, really great job on the whole in writing out all the JS here! 😊

  10

  Marked as helpful
• 

  P

  John Mirage•1,590

  @john-mirage

  Posted almost 3 years ago

  Well done!

  A personnal tip that you may find interesting (or not):

  Selectors

  • I always use different selectors for CSS and JS.
  • I always use classes for CSS to have the same specificity. MDN - specificity

  Classes for CSS

  this example use the BEM methodology (recommended).

  HTML

  <ul class="list">
    <li class="list__item">
      <a class="list__link list__link--blue">home</a>
    </li>
    <li class="list__item">
      <a class="list__link list__link--red">about</a>
    </li>
  </ul>
  

  SCSS

  .list {
    margin: 0;
    padding: 0;
    display: flex;
    flex-direction: row;
    align-items: center;
  
    &__item {
      padding-left: 1rem;
      padding-right: 1rem;    
    }
  
    &__link {
      text-decoration: none;
      color: green;
  
      &--blue {
        color: blue;
      }
  
      &--red {
        color: red;
      }
    }
  }
  

  data attributes for JS

  Using id is great when you only have one element. But when you need to select multiple elements, you need to use a class wich is only used for CSS. Data attributes can be used instead of classes and ids to select elements with JS. more info here

  Make sure that the data-id used for only one element is unique on the page (like an id);

  HTML

  <main class="app" data-id="app"></main>
  <ul class="list">
    <li class="list__item" data-id="list-item"></li>
    <li class="list__item" data-id="list-item"></li>
  </ul>
  

  JS

  const appElement = document.querySelector('[data-id="app"]');
  const listItemElements = document.querySelectorAll('[data-id="list-item"]');
  

  Why it may be better:

  • By using only classes for CSS, we avoid specificity issues.
  • By looking at our code, we now know which elements are selected with javascript and wich are not.
  • We can change a class without breaking a javascript selector. they are not related.
  • We can change a data attribute without breaking CSS.

  11

  Marked as helpful