Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tightly coupled components refactory #42

Open
EnricoBarbieri1997 opened this issue Jun 21, 2022 · 2 comments
Open

Tightly coupled components refactory #42

EnricoBarbieri1997 opened this issue Jun 21, 2022 · 2 comments
Assignees
Labels
enhancement New feature or request

Comments

@EnricoBarbieri1997
Copy link
Collaborator

Components (in particular the ones inside BottomSheet) needs to be refactored because some are tightly coupled with stores, outer most components and BottomSheet. For the ones tightly coupled with stores or controllers is enough to have a version where everything is a prop and a version that calculates the current logic and passes it down as prop.
Not every component that access stores or models directly needs to be changed but there are a lot of general components that can't be reused because instead of having generic event handlers they have a very specific model passed to it

@EnricoBarbieri1997 EnricoBarbieri1997 added medium priority After all high priority tasks are closed or stalled and removed medium priority After all high priority tasks are closed or stalled labels Jun 21, 2022
@EnricoBarbieri1997
Copy link
Collaborator Author

I will provide an initial list with examples of which ones are fine and which ones are not as soon as possible

@giorgionocera giorgionocera added the medium priority After all high priority tasks are closed or stalled label Jun 21, 2022
@giorgionocera giorgionocera removed their assignment Jun 21, 2022
@EnricoBarbieri1997
Copy link
Collaborator Author

Examples of it are:

  • PinCode: it takes as props a Pin class instead of a general value, onChange pair. We could have a base component that just takes the value and the event and another one that builds on top of it that uses this Pin class. If you do want to have 2 events for press and remove it's fine. The problem here is not the events subdivision but the dependency on Pin class.
  • Footer files: there are at least a couple of footer files (with slightly different style) that are inside the screens they are used into. Why don't we move them out? They have similar functionalities and could be used elsewhere in the future.
  • SendRecap/Recap: takes as input a controller/creater. I'm not sure about this paradigm. It's the first time that I've encountered it. Mixing multiple inputs prevents passing a lot of props around but makes inner components cryptic as they don't explicitly list their props.

zheleznov163 added a commit that referenced this issue Jul 7, 2022
zheleznov163 added a commit that referenced this issue Jul 7, 2022
zheleznov163 added a commit that referenced this issue Jul 7, 2022
@EnricoBarbieri1997 EnricoBarbieri1997 added low priority After all medium priority tasks are closed or stalled enhancement New feature or request and removed medium priority After all high priority tasks are closed or stalled low priority After all medium priority tasks are closed or stalled labels Sep 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants