Interactive Ratings Component

Community feedback

• 

  Reddsito•210

  @ReddsitoPosted about 2 years ago

  Hello, I will not be the most experienced but, the way I make the change is simply that, when an option is already selected, when pressing the "submit" button add a class to the main box, for example "--active" and I make that when that box has that class, do display: none to all the div, except the one I want to show, I don't know if it's more practical but it's an option :D

  1
• 

  Amélie•330

  @aweliegoPosted about 2 years ago

  Hi Benny,

  There are many, many ways to accomplish this with vanilla JavaScript and CSS. Part of what one might consider a solution more correct than another might be the side effects caused by the chosen method. Mostly, with the 'position absolute' method (which I've also used, btw) is that the element with position absolute will remain visible in the DOM, but this harms accessibility as absolute positioning overrides the default flow layout of browsers. From what I understood, this would also be the case if you would try to tweak the visibility/opacity or use transform properties (such as translate() or scale()) to make the element disappear from sight. [Please someone correct me if I'm wrong]. From what I've found, only changing the display from block to none will effectively remove the element from the DOM and thereby provide a better experience for all users. The WebAIM provides a good thought starting point on the topic: https://webaim.org/techniques/css/invisiblecontent/

  That being said. Targeting the display property won't allow for CSS transitions (in case you'd want to add some to switch from one card to another) as it is not possible to animate/transition from display: block to display: none. Therefore I found it is tricky to accommodate accessibility and transitions, but I'm sure there are ways to do this, just haven't looked for them at this point.

  Regarding your solution for this particular point: I'm not sure about the necessity to position the rating tool card absolute. If I remove that property in the dev tools, it seems to be hiding the attribution div, but if you delete that one you have no need for the absolute position anymore.

  Some more feedback:

  • I think the colour scheme for the buttons on hover/click is reversed: they should actually be greyed out when you hover over them and the one that is click should change to (and remain) orange :)
  • I would make the cursor a pointer when hovering over the rating and submit buttons

  Other than that, your code looks quite DRY and you used both Grids and Flexbox which is interesting - good job on this challenge!

  0

  ---

  

  Benny Heo•30

  @bennyheoPosted about 2 years ago

  Thanks for the feedback. I see your point on using absolute positioning. It seemed like the quickest way to achieve what I was going for but in the future I will try other methods for stacking the two objects.

  I completely forgot that the attribution was there. I moved it down so that it's visible now.

  The button colors seem to be correct based on the design images.

  The cursor looked like pointers from the browsers I used but I went ahead and assigned it just in case it looks different on other computers. This is probably something I should do regularly.

  0