search

gnosis / ido-ux

Interface for the ido-platform
GNU General Public License v3.0
25 stars 22 forks source link

Feedback on the app #617

Open maria-vslvn opened 3 years ago

maria-vslvn commented 3 years ago

The whole app is really interesting and usefull. The design look modern, minimalistic and clear. I like the codestyle in general, there are only a few things i could suggest to make the code easier to read and make easier to get to the component code itself.

  1. Keep styled-components code in separate files. It would make it easier and faster to get the main code of the component itself.
  2. Create global style rules for components and keep it separately.
  3. As we have a lot of really similar elements, we could create a file or several files that would have some common things. For example we could create a Wrap or Button elements and just pass there all the neccessary props. I had something like in one of my previous projects where i'd just created one element Wrap, that had a lot of props with their default values for the cases when i didn't set the prop value. Also we can add the possibility of responsive view, for example write the prop value as an array with string values, where the 0 value is for mobile, the 1 is for tablets and so on. We can also add some logic like: make flex-wrap: wrap when flex-direction: row and flex-wrap isn't setted explicitly. So the main idea is to make more common and reusable elements in styled-components.
  4. In case of further development and scaling the app we could add the react-intl library in order to keep translations.
josojo commented 3 years ago

Great. Thanks for the feedback!

Yeah, we could tackle 1. and 2. over time. If you wanna introduce this already for your new PR for the new code, I would for sure approve it and the rest could be refactorerd over time.

As you think its best. I don't have strong preferences.

ramirotw commented 3 years ago

Really good points @maria-vslvn !

I'm still digging into the codebase so my observations could not be that precise at this moment, but here are my thoughts:

  1. To tackle this in other projects what we have done is having the styled components defined at the bottom of the file and not on the top as they are now. Regarding moving them to individual files, it would make sense if: 1) the file is getting too big and we may introduce git conflicts when is a common component updated regularly; 2) the component is reused in another file.
  2. It's not crystal clear what you meant by this, could you expand with an example?
  3. I totally agree on this one. We used https://styled-system.com/ in another project which handles a lot of what you mentioned.
  4. The project already has i18next configured but is not used. I believe it's just a matter of replacing strings at this point on each file.