registry dapp #2077
registry dapp #2077
Conversation
|
It looks like @derhuerst hasn'signed our Contributor License Agreement, yet.
You can read and sign our full Contributor License Agreement at the following URL: https://cla.ethcore.io Once you've signed, plesae reply to this thread with Many thanks, Ethcore CLA Bot |
|
[clabot:check] |
|
It looks like @derhuerst signed our Contributor License Agreement. 👍 Many thanks, Ethcore CLA Bot |
|
@jacogr |
| }; | ||
|
|
||
| const { name, key } = this.state; | ||
| const props = this.props.lookup; |
There was a problem hiding this comment.
Would have preferred if it was const lookup = this.props.lookup since it just clarifies exactly what you are working with.
There was a problem hiding this comment.
Yeah. I'm definitly not happy with <Lookup lookup={ lookup } actions={ actions.lookup } />. Would like to pass the props in directly without mentioning every single one. Is there a way?
There was a problem hiding this comment.
Look at Tomasz' comment above, you can do <Component {...this.props} /> to pass everything available.
Now, a reducer can process an action that is not strictly related to the reducers area of responsibility, which is actually how Redux proposes to couple components. See http://redux.js.org/docs/basics/Reducers.html
In lookup/reducers, I changed `key` to `entry` because `key` is a special React prop.
This has a caveat: React will complain in the beginning because the initialState oftern contains `null` as default value, with means "missing" for React.
|
I updated a few things. Left to review are the discussions on |
|
|
||
| const renderDropped = (e) => ( | ||
| <p key={ e.key } className={ styles.dropped }> | ||
| { renderOwner(e.parameters.owner) } |
There was a problem hiding this comment.
I think I would prefer JSX instead of function calls here:
<Owner owner={e.parameters.owner} />
const RenderOwner = ({owner}) => ( ...unchanged... );
For me it would be easier to scan throught structure more quickly: It would be just another component displayed along with abbr and code tags.
A matter of taste, though.
tomusdrw
added
A6-mustntgrumble 💦
|
👍 lgtm modulo minor style grumbles |
We might actually consider to pull this out into a more general component, as `ui/Form/InputAddressSelect` is focused on a form input.
|
I'm sorry for making this PR so huge 😢 I'm considering the Registry dapp working well enough to deploy it. Therefore, I'd like to make all further changes (except minor grumbles) in follow-up PRs. Please review the last parts (account selector, events list). |
derhuerst
added
A0-pleasereview 🤓
jacogr
added
A8-looksgood 🦄
|
Maintanable, good as a first stab to start getting it tested and fed back on. |
This PR adds a basic Registry dApp that allows users to query
Registry.sol.Even though the app is not done yet (in fact, a lot is missing), it serves as an example to discuss code style with the JS team.