DEV Community

Moran Helman
Moran Helman

Posted on

Can you help me with some feedback on my react practice code, please?

I could really use some feedback on this assignment,
this was a home code interview task, my feedback was: "it's not that good, your level is not sufficient"

Is it?
i want to know how can i improve for next time.

working example:
https://csb-g81mx-du0znf2sl.now.sh/

code:
https://github.com/send2moran/test

Thank you!

Top comments (4)

Collapse
 
emma profile image
Emma Goto 🍙

It's hard to know the exact reasons why - I think a good first step is to have Eslint set up so that your code is styled nicely. If you look inside of PhotoGallery.jsx some of the lines of code in there are really long.

Other things I noticed in that file:

  • You had some inline onlicks - onClick={e => {e.stopPropagation(); handleDelete(p.id)}} I think would have been better to extract these into proper functions like you did for other things
  • You had a <h1 title={p.title}>{p.title}</h1> - not sure if that's a Tailwind thing, but do you. need to be passing it in more than once?

In Modal.jsx:

  • Both of your usages of ModalPop passed in the same null argument, maybe you could have skipped passing it in if it's always going to be null. And rename it to onCloseModal

I think adding some unit tests also goes a long way.

Collapse
 
send2moran profile image
Moran Helman

amazing ! thank you for the feedback❤️

Collapse
 
pieterbergwerff profile image
Pieter Bergwerff

Did your assignment require using Redux? Otherwise I would say it's maybe a better choice to use React state, context / hooks for managing Gallery Viewer state?

Also, I see in your code, for example in index.js: <h1 className="uppercase text-6xl text-blue-900 font-bold leading-none tracking-wide m-10">Simple Gallery ({state.albums.length})</h1>. I would create a seperate h1 component for it, your code becomes more readable.

Success on the new job!

Collapse
 
send2moran profile image
Moran Helman • Edited

unfortunately i do, i use Mobx and MST when i need state management :)
and now, mobx and useContext looks very promising.

Thanks :)