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

Refactor HeaderSections #149

Open
debragail opened this issue Dec 16, 2018 · 7 comments
Open

Refactor HeaderSections #149

debragail opened this issue Dec 16, 2018 · 7 comments

Comments

@debragail
Copy link
Contributor

Currently HeaderSection includes:

Last Block
RPC Server
Transactions
Mining Status

../src/reducers/headerSections.js

screen shot 2018-12-15 at 22 52 32

It might make sense to handle this container differently.

Expected Behavior

Current Behavior

The state for all 4 classes are handled as HeaderSections and it might be better to refactor this now.

Possible Solution

Handle HeaderSections as a header with individual components instead of together in headersections

Steps to Reproduce (for bugs)

Context

Trying to update the Xplore page to have results and be more modular in regards to state and clickable links.

Your Environment

@bicep
Copy link

bicep commented Dec 18, 2018

Hello!

Handle HeaderSections as a header with individual components instead of together in headersections

My plan of attack: 4 different reducers in the reducers folder and then 4 different components within the HeaderSections.jsx file.

Does this make sense?

Actually I thought the original writer of the code wanted to just put it all together so that each of the sections could be iterated/mapped through. Am I right to say that you are requesting this change now because each of the header sections are getting more different from each other and should not be treated in the same way?

@TalAter
Copy link
Member

TalAter commented Dec 18, 2018

I also think it makes sense to refactor it like that... this attempt at over generalization could be simplified simply by having 4 components - and if there are similarities between all of them, we can simply have all 4 extend another component.

@bicep would you like to take this one after you handle the other task?

@debragail
Copy link
Contributor Author

Also take into consideration that for search results page we will need a page to return for each section.

@bicep
Copy link

bicep commented Dec 22, 2018

@TalAter @debragail Yeap let me give this one a shot.

@debragail
Copy link
Contributor Author

@bicep let me know when you're ready to tackle this and I can write the corresponding tests.

@bicep
Copy link

bicep commented Jan 23, 2019

Yep, will do! I may give it a shot this weekend.

@mariolo1985
Copy link

Hey guys - may I suggest writing any React components that does not use state as a functional component. This will be more pure without needing to go through additional lifecycles pass render. You can also destruct props and know what arguments you are using.

Please correct me if I am interrupting the implementation wrong. One thing I saw on the Redux side is that reducers should not be passing in a hardcoded state. It should use the state declared for the store. If ever a property needs to be updated in the hardcoded state, this will override the update.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants