Interactive Rating Component using vanilla JavaScript

Community feedback

• 

  

  Grace•32,130

  @grace-snow

  Posted over 2 years ago

  Hi

  There are still quite a few issues in this, but it is much improved from before.

  I recommend reading up on modern-css.dev about how to accessibly style radio inputs. What you've got at the moment is not accessible and is far more complicated than it needs to be.

  I also see issues with how you are using explicit heights and widths; and margin/padding.

  This component should not have a height or width. Just a max-width in rem. NO height at all. Let the browser dictate the height needed based off the content inside.

  With padding and margin, remember the box model and keep it simple. All elements inside the component don't touch the edge of the box. So use padding on that box - padding is for internal spacing and does not collapse. For the elements inside the box (heading, paragraph, form...), you want to create space between those elements, so use vertical margins - margin is for external spacing and does collapse.

  The other point which I think has been raised above is how to center your component on the page. Don't use huge margins. Use flex/grid properties along with min-height 100vh on the wrapping element to center it's contents on the screen.

  All the relative/absolute positioning is unnecessary on this. You may use some for the radios but the markup structure would need to change first (see that article I mention above)

  
  /* ratingcomponent.css | https://www.clewisdev.com/FMinteractiveratingcomponent/ratingcomponent.css */
  
  section {
    /* width: 327px; */
    /* height: 360px; */
    /* margin: 0 auto; */
    /* margin-top: 9.625rem; */
    max-width: 20.4rem;
    margin: 1rem; // stop component hitting screen edges
    padding: 2rem;
  }
  
  body {
    min-height: 100vh;
    margin: 0;
    padding: 0;
    display: grid;
    place-content: center;
  }
  
  .star {
    /* position: relative; */
    /* top: 1.5rem; */
    /* left: 1.5rem; */
    /* margin-bottom: 1.875rem; */
    margin-bottom: 1rem;
    display: block;
  }
  
  .title {
    /* padding-left: 1.5rem; */
  }
  
  .subtitle {
    /* padding-top: .4375rem; */
    /* padding-left: 1.5rem; */
    /* padding-right: 1.5rem; */
    margin-top: 1rem;
  }
  
  .input-div {
    /* padding-left: .5rem; */
    margin: 1rem 0 2rem;
  }
  
  [type="radio"] {
    note: this is not how you accessibly hide inputs! opacity 0 is not right;
    note: needs required attribute on one radio in html to prevent submit before completion;
    clip: rect(1px, 1px, 1px, 1px);
    clip-path: inset(50%);
    height: 1px;
    width: 1px;
    margin: -1px;
    overflow: hidden;
    padding: 0;
    position: absolute;
  }
  
  [type="radio"] + label {
    note: wrap the input in the label and style the label;
  }
  
  label {
    /* margin-top: .5rem; */
    /* margin-bottom: 2rem; */
  }
  
  [type="radio"] + label::before {
    /* content: ''; */
    /* position: absolute; */
    /* left: -16px; */
    /* top: -14px; */
    /* width: 42px; */
    /* height: 42px; */
  }
  
  .submit-btn {
    /* position: relative; */
    /* left: 2rem; */
    /* width: 85%; */
    /* margin-bottom: 2rem; */
    width: 100%;
  }
  

  13

  Marked as helpful
• 

  

  Grace•32,130

  @grace-snow

  Posted over 2 years ago

  I'm afraid the html is incorrect on this. It must be a form with a fieldset of radio inputs so that the ui works as expected and state is communicated for keyboard and screenreader users etc. It cannot be a load of divs. NEVER ever add interactive behaviour to non-interactive elements. I cannot stress how important this is.

  18

  Marked as helpful
• 

  Sinka László•470

  @dlxzeus777

  Posted over 2 years ago

  Hey C Lewis nice looking solution, however you could've just used a for of loop to loop through the buttons and get its value and apply it to the innertext.

  Like this for example:

  for(let btn of buttons) { btn.addEventListener('click', (e) => { const target = e.target.value; console.log(target); selectedNumber.innerText = target; }) }

  11
• Account deletedPosted over 2 years ago

  Hey there! 👋 Here are some suggestions to help improve your code:

  • To properly center your content to your page, you will want to add the following to your <Body> Element (this method uses CSS Grid):

  body {
      min-height: 100vh;
      display: grid;
      place-content: center;
  }
  

  • Change width to max-width in your component’s container to make it responsive. You will also want to delete the height as it is unnecessary.

  • The proper way to build this challenge is to create a Form and inside of it, the “rating buttons” should be built using an Input Radio and it should have a Label Element attached to it.

  If you have any questions or need further clarification, feel free to reach out to me.

  Happy Coding! 🍂🦃

  00