search

lakshyakanungo / neeto-ui-challenge

https://wheel-production.neetoreviewapp.net/
MIT License
0 stars 0 forks source link

Made review changes #21

Closed lakshyakanungo closed 1 year ago

lakshyakanungo commented 1 year ago

Fixes #19

@yedhink @navaneethsdk _a Please review.

lakshyakanungo commented 1 year ago

@navaneethsdk I have written this line since Eslint was giving an error that we can't have unused variables and it was not allowing me to commit. https://github.com/lakshyakanungo/neeto-ui-challenge/blob/2ab2a79bdc07fd1f4a98054d3579fc43f5a9d344/app/javascript/src/components/Dashboard/Contacts/index.jsx#L31

lakshyakanungo commented 1 year ago

Some things in my mind

  1. Do we need to mention the button type=“button” if its not type submit or reset?
  2. We should name event handlers as “handleEvent“ where the event handler function is defined.
But while passing them as props to our custom component it is better to name the prop as onClick/onClose/onSubmit right?
Like I’m passing this event handler to Form component using onClose prop instead of handleClose.

https://github.com/lakshyakanungo/neeto-ui-challenge/blob/2ab2a79bdc07fd1f4a98054d3579fc43f5a9d344/app/javascript/src/components/Dashboard/Contacts/Pane/Create.jsx#L13C1-L26

  1. Snake case is being used at a few places in react best practices for localisation. That’s why I thought it might be allowed in translations.
 https://courses.bigbinaryacademy.com/learn-react/react-best-practices/locale-translations-in-react/
lakshyakanungo commented 1 year ago

Title of issue: Deploy application

Deployed at https://main-app.neetodeployapp.com/notes Can be logged in using: Username : oliver@example.com Password : welcome