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

Closes #14, closes #15; Create dapp to register new owner, vehicle #23

Merged
merged 1 commit into from Mar 18, 2018
Merged

Conversation

ghost
Copy link

@ghost ghost commented Mar 11, 2018

Closes:
#14 Create dapp to register new owner
#15 Create dapp to register new vehicle

Changes proposed:

  • Implements forms for user input and handles related contract logic
  • Replaces Truffle box resources with UIKit

@michalmikolajczyk I'd appreciate your feedback

.then((result) => {
const state = { ...this.state };
const machine = {
model: model,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shorthand:

const machine = {
  model,
  make,
  vin,
  address: result.logs[0].args.newMachine
};

})
}

callCreateMachine() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this function does not return anything. I would suggest returning a value when defining functions.

form._disabled = !isValid;

// Update state
const state = { ...this.state };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const { state } = this;

<div className="uk-container uk-container-center uk-margin-top uk-margin-large-bottom">
<div className="uk-grid-match" data-uk-grid-margin>
<RegisterOwnerForm
onSubmit={(e) => this.ownerFormSubmittedHandler(e)}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

onSubmit={this.ownerFormSubmittedHandler}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you would need to bind it to this in the constructor first. Add to the constructor something like:

this.ownerFormSubmittedHandler = this.ownerFormSubmittedHandler.bind(this);

for all functions that refer to this, obviously. That is very handy when passing the functions to child components.

<RegisterMachineForm
onChange={(e) => this.machineFormChangedHandler(e)}
onSubmit={(e) => this.machineFormSubmittedHandler(e)}
state={this.state.forms.registerMachine}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you refer to this.state more than once, it would be worth it to deconstruct it at the top of the function

const { state } = this;

or

const { forms, contracts } = this.state;



export default registerOwnerForm;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!

@michalmikolajczyk
Copy link
Member

Basically the work looks really cool!

  1. I would suggest we refer to Airbnb style guide https://github.com/airbnb/javascript for both JS and React. I will add an eslint file.
  2. I am not sure if we should load the full ui-kit package. We don't need the rtl file, and I don't really see us modifying the package – perhaps the minified files would suffice?
  3. we need to have proper unit test coverage of all files with business logic

@michalmikolajczyk
Copy link
Member

in fact, can we use ui kit from npm?

@michalmikolajczyk michalmikolajczyk merged commit 20ce606 into blockchain-IoT:master Mar 18, 2018
@michalmikolajczyk
Copy link
Member

I applied another commit, now we use ui kit from npm.

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

Successfully merging this pull request may close these issues.

None yet

2 participants