TodoAPP made with React.js , Sass, Jsx

Community feedback

• 

  Raymart Pamplona•16,100

  @pikapikamartPosted about 3 years ago

  Hey, really great job on this one. Also when I tackled this challenge , it is my first time using react as well so really great job on this. The site looks fine but I prefer the todo-item container to be shrink when there are no items yet.

  Carla already great great tip or idea on this one, just going to add some suggestions as well:

  • Avoid using height: 100vh on a large container like the body tag as this limits the element's height based on the remaining screen's height. Instead use min-height: 100vh, this takes full height but lets the element expand if needed.
  • Also you don't need width: 100% on the body since by default, block element uses 100% of the width.
  • Do not directly type the wordings as uppercase on the markup, if you do this, screen-reader will read the text letter-by-letter and not by the wordings. Use only the lowercase version to write in the markup and instead use text-transform: uppercase on it.
  • Using img alone for the toggle is not really great. Interactive components uses interactive elements. By using img you are making it not-accessible.
  • Now on the toggle, the proper markup for that is to use 2 radio buttons which is inside a fieldset tag with a screen-reader only legend element. This is really accessible btw which I implemented on my own solution for this. Take note of the structuring and the usage of the label for the input.
  • Always have a main element to wrap the main content of your page. You could wrap the form inside the main but on my solution, I made it all main for the whole layout since I think it is better that way.
  • The circle inside the input is not really a separated component. It should be just like a :before it is only decoration on the input to be honest.
  • The main input should use aria-label attribute or screen-reader element inside. The value will describe what does the input element needs since like using a input todo item.
  • You could add an aria-live element as well that announces the addition of the todo-item that the user had made. Just an extra idea for accessibility.
  • For every todo-item, it should have an input type="checkbox" since users will check if either they want the todo to be completed or not. The todo-item text should be the label for the input so that the user could toggle it. Right now there is no way to toggle the todo-item without using the mouse, the remove the todo-item as well is not using a proper element, use button for that since it is interactive component.
  • Those 3 selection on the bottom should either be a list of button using aria-live that announces if a certain option is selected or a set of radio buttons inside a fieldset with a screen-reader only legend element and not p tag like what you are using now.
  • When wrapping a text-content do not just use div to wrap it, use meaningful element like a p tag if it just a regular text or heading tag if it is an heading.
  • clear completed should be a button.

  Right now there are lots of improper use of html elements. When going into libraries like react, first, you must have a proper and great fundamentals about semantic html elements. If you are not ready don't go with the trend first, strengthen your fundamentals first, because the last thing you want to create is a not-accessible application.

  But still, great job again on this one.

  Marked as helpful

  1

  ---

  

  Francisco•145

  @FrankiiizePosted about 3 years ago

  @pikamart thx for the tips you are make me to get better , i'm improving the changes, i just no sure how to implement the accessibility, im reading it now =( but, its important you're right , i change the tags for the buttons and redo the css to match to the desing , im no sure about the filedset tag, ill try to change it, ill try another attempt. thx a lot for your time i apreciated it.

  PD: my english is no good i'm sorry if my grammatical are bad e.e! changing the bottom filter for buttons tags i realize that i could disable the buttons when are loading that's a plus !

  1
• 

  Carla•420

  @iamcgsPosted about 3 years ago

  Nice job! Works well on my end. The only thing i'd point out is that the bottom menu (where the filters are) in mobile does not look good. The options "All, Active and Complete" are not centered. Also, the text "Drag and drop to reorder list" needs some top-margin in my opinion. But the functionality es great. Congrats!

  Marked as helpful

  0

  ---

  

  Francisco•145

  @FrankiiizePosted about 3 years ago

  @iamcgs oops! oops! thx i notice the bug, if you resize the navigator window the styles broken, but if you refresh the page after resize the window the styles works, ill check that ! i kw what is happening :D, thx a lot!

  1