-
Notifications
You must be signed in to change notification settings - Fork 1
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
Frontend/#043 button and link components #56
Conversation
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.
In my opinion it's the seed to spaghetti code. I find this code way more difficult in usage and it doesn't bring anything more than standard <button>
element.
function Button({ textToDisplay, classes, callback }) { | ||
return ( | ||
<button className={classes} onClick={callback}> | ||
{textToDisplay} | ||
</button> | ||
); | ||
} |
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.
don't you think it's way more complicated to use buttons like this?
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.
Could you suggest any specific solution to this problem?
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.
@synowa just use buttons as standard jsx element. There's no need to extract it to a separate component right now.
Let's say:
<codeBefore/>
<button anyPropsINeedRightNow>anyText</button>
<codeAfter/>
Pros:
- No imports (less messy components directory in the future)
- Every additional prop that's not defined in
Button
component would have to be declared
If your not able to define how many props you'll need then you should use the spread operator
const Example = (props) => (
<someElement ...props>{props.children}</someElement>
);
props.children stands for any child you put inside this component, eg. text or another component
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 actually had in mind a very specific button, with specific style and function - a go back button, using props.history.goBack(). But I didn't have time to implement it yet. So far yes, it is reduntant.
closes #43