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

Make Button examples run a user-defined function #21086

Closed
dandv opened this issue May 17, 2020 · 6 comments · Fixed by #21234
Closed

Make Button examples run a user-defined function #21086

dandv opened this issue May 17, 2020 · 6 comments · Fixed by #21234
Labels
component: button This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation ready to take Help wanted. Guidance available. There is a high chance the change will be accepted

Comments

@dandv
Copy link
Contributor

dandv commented May 17, 2020

This may seem totally obvious to those experienced with MUI, but to those totally new to it, it may come as a surprise that none of the Button pages (Button, ButtonBase, IconButton) mention or show an example of how to actually initiate an icon as a result of clicking a button. 😄

This is textbook example of the curse of knowledge.

There's the implicit assumption that buttons extend the HTML button elements, so onClick should be set to a function, but for new users, that may just not be obvious. Not even the ButtonBase API page mentions anything about onClick, or extending the button element.

If the above makes sense, the examples on the Button* pages could be improved to allow a user brand new to MUI, to get started with a button performing an action. MUI may be the jQuery of building web apps, and it's reasonable to anticipate that some developers may simply not have the necessary HTML background assumptions.

@oliviertassinari
Copy link
Member

oliviertassinari commented May 17, 2020

@dandv This topic has come up in the past. So far our position has been that any none documented prop is forwarded. Developers can expect the same behavior than from native elements when using the Material-UI's components. I would be in favor of keeping the current tradeoff as the best worse alternative and move the discussion to #18288 for a better solution.

Curse of knowledge

I wish I could unlearn all I know about the framework. @dtassone I count on you to report things you found weird/hard to grasp as still relatively new to the library. For instance, you have mentioned the styles behind hard to grasp during our last meeting. It's likely something worth diving into.

@eps1lon
Copy link
Member

eps1lon commented May 18, 2020

I think this is not necessarily related to #18288. The primary use case for a button is to handle a click. Not showing that in the demo can't be excused with "the prop is implied in the docs" not even with a future "the prop is explicitly listed (among 10 other props)".

Generally I feel our docs are lacking in displaying interactions. We oftentimes use alert which is too disruptive in my opinion. Would be nice if we could improve this (e.g. transpiling alert to internal tools such as stacking stacking snackbars) and then we have a good tool for displaying button interactions.

I'd even go as far as making onClick required but that's for another day.

@eps1lon eps1lon added component: button This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation and removed duplicate This issue or pull request already exists labels May 18, 2020
@dandv
Copy link
Contributor Author

dandv commented May 23, 2020

I've just read the API Design Approach page, which has this to say about Native properties

We avoid documenting native properties supported by the DOM like className.

Now the lack of onclick is consistent, and makes sense.

While I should have read that page before reading any individual components or API doc pages, I feal other users may also skip it. Not sure I have a good suggestion for this, other than suggesting on every single page that the user read that first, which may be overkill. Or having the very last linked base element be the underlying HTML element? E.g. at the bottom of the Button page, add HTMLButtonElement (and actually reorder the 4 itmes lke this, Button, IconButton, ButtonBase, HTMLButtonElement).

@oliviertassinari
Copy link
Member

@dandv Is this problem worth solving? I feel that it would only affect users very new to React, which will eventually figure it out.

@eps1lon
Copy link
Member

eps1lon commented May 24, 2020

It's the default use case of a button. Not illustrating how this works is an issue that's worth solving. It goes beyond "how do I pass my handler" e.g. when does it activate?

@oliviertassinari
Copy link
Member

@eps1lon So we move forward with an onClick handler/section in https://material-ui.com/components/buttons/? :)

I'd even go as far as making onClick required but that's for another day.

I would be cautious with the link use case of the button.

@oliviertassinari oliviertassinari added the ready to take Help wanted. Guidance available. There is a high chance the change will be accepted label May 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: button This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation ready to take Help wanted. Guidance available. There is a high chance the change will be accepted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants