-
Notifications
You must be signed in to change notification settings - Fork 120
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
Functional proposal details #2449
Conversation
fdd13d4
to
d4c590b
Compare
d4c590b
to
3538fa6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vctt94 nice work mate!
Added some small suggestions, mainly to declare components as arrow functions and use destruction for props when declaring a functional components which should improve the readability slightly.
} | ||
} | ||
|
||
function ChooseVoteOption({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
function ChooseVoteOption({ | |
const ChooseVoteOption = ({...}) => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here, as it is a component, I'd rather let it as function. Otherwise, as an arrow function it does not show at the stack trace, when erroring, which makes debug harder.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, makes sense
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
eligibleTicketCount,currentVoteChoice, showPurchaseTicketsPage | ||
}) => { | ||
// getVoteInfo is an auxiliar function to get the properly vote info component. | ||
function getVoteInfo () { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
arrow function ?
/> | ||
);} | ||
|
||
function getError(error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about here ?
54407c5
to
0d921f2
Compare
Changed. Thanks for reviewing |
0d921f2
to
1c35c99
Compare
This PR depends on #2448.
It makes proposal details a functional component. With that we can remove our
proposal
connectors