Skip to content
This repository has been archived by the owner on Apr 27, 2023. It is now read-only.

Refactor Saka search modes to use React best practices #69

Open
pureooze opened this issue Aug 10, 2018 · 3 comments
Open

Refactor Saka search modes to use React best practices #69

pureooze opened this issue Aug 10, 2018 · 3 comments
Labels
maintenance This issue is not a feature but instead some refactoring that improves the code quality
Milestone

Comments

@pureooze
Copy link
Member

Saka currently relies on mode specific providers and "dumb" components to render search results for the selected mode. We currently define a "dumb" component for each search mode. This is done so that we can set certain things like the icon and color for each suggestion as they differ between each type of suggestion.

We should refactor Saka to have a single generic suggestion component that accepts props that define all the suggestion specific parameters.

The refactoring will make the code a lot easier to understand and maintain.

Some related docs:
https://reactjs.org/docs/composition-vs-inheritance.html
https://reactjs.org/docs/components-and-props.html

@pureooze pureooze added the maintenance This issue is not a feature but instead some refactoring that improves the code quality label Aug 10, 2018
@pureooze pureooze changed the title Refactor Saka search modes to use React/Preact/CBA best practices Refactor Saka search modes to use CBA best practices Aug 10, 2018
@pureooze pureooze changed the title Refactor Saka search modes to use CBA best practices Refactor Saka search modes to use React best practices Aug 10, 2018
@eejdoowad
Copy link
Member

Just FYI, I’ve already begun and mostly completed this in the branch remove-mwc. It still has quite a bit of work to be done, but I’ll get something PR worthy in by Saturday.

@pureooze
Copy link
Member Author

Awesome 😃 ! Don't feel rushed to get it in, I think this type of refactor definitely benefits from careful evaluation of all the possible solutions.

@pureooze pureooze added this to the 0.17.0 milestone Aug 27, 2018
@eejdoowad
Copy link
Member

So.... I got sidetracked and never got a chance to work on this, and probably won't for a while. I've pushed what I had at the time to remove-mwc. It may or may not be useful.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
maintenance This issue is not a feature but instead some refactoring that improves the code quality
Projects
None yet
Development

No branches or pull requests

2 participants