Code review - React, axios

Code Feedback
1

Hi!
I’ve built a project in React. On init you should see a list of people. You can RESET list or ADD new people to the list.
Clicking on person from list should show you a details about this person, you can EDIT these details, when editing is done, you can SAVE details and then you’ll be redirected to page with details.

Any edited and saved data is storaged in IndexedDB. If there is a information already storaged in IndexedDB additional data won’t be fetched.

I’ll be glad to here your opinions.
Thanks in advance.

Demo: List of people
Code: GitHub - Dariuszb94/PersonsList

2

Took a quick look and some things that caught my eye:

  • Either debounce mouse position update or get rid of it altogether (in usePosition.js), because of performance issues.
  • Inconsistency in styling - some files are CSS, some SCSS; class naming that resembles BEM, but is not.
  • Dexie with it’s 79kB seems a bit on the heavy side compared to alternatives like idb-keyval (which is 883B) :man_shrugging:
  • DB logic probably should be moved outside the component.
  • “Save” button is in Polish.
  • If the name in the list has non-latin characters (e.g. Mr بردیا کامروا, or even with something like Ś), your name validation RegEx fails.
  • “Reset” should probably also reset selected name in the list (to first).
  • There are very few cases when you should use !important (I don’t think yours is one of those).
  • Why handleSubmit in Edit.js is marked as async?
  • What is the point of else statement in handleSubmit?
  • I think that the Router is not really needed for this app.
  • If there are more than 15 names in the list, not all names are shown, but maybe it’s by desing :smirk:
  • Inital fetching logic seems a bit convoluted, but I didn’t look too much into it, so maybe I’m wrong.
3

First of all I want to thank you for such meaningful review of my code :slight_smile: Below are my replies to your feedback

Either debounce mouse position update or get rid of it altogether (in usePosition.js), because of performance issues.

You are right, now I’ve added debounce to it.

Inconsistency in styling - some files are CSS, some SCSS; class naming that resembles BEM, but is not.

Not all styles were scss, because I used scss only if it is needed. Now all files are scss even if there is no need to use scss in them. Have you any guesses to improve namings?

DB logic probably should be moved outside the component.

You sure it is necessary?

“Save” button is in Polish.

Well… yeah, I have overlooked this.

If the name in the list has non-latin characters (e.g. Mr بردیا کامروا, or even with something like Ś), your name validation RegEx fails.

Now I have fixed it by using different regex, now it works fine.

There are very few cases when you should use !important (I don’t think yours is one of those).

I have overlooked it.

Why handleSubmit in Edit.js is marked as async?

You are right, it was not necessary

What is the point of else statement in handleSubmit?

You are right, there is no point, I have overcomplicated it

If there are more than 15 names in the list, not all names are shown, but maybe it’s by desing

Thank you for see this, I didn’t notice that it was broken by other style. Now it is fixed

Inital fetching logic seems a bit convoluted, but I didn’t look too much into it, so maybe I’m wrong.

Have you any guesses to improve this logic?

4

This topic was automatically closed 182 days after the last reply. New replies are no longer allowed.