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

[withWidth] Document the render prop #13074

Merged

Conversation

JulienUsson
Copy link
Contributor

@JulienUsson JulienUsson commented Oct 2, 2018

Hello,

This PR allows user to rename withWidth props like asked in #13074
You can rename props with renameProps.
Do you think others HOC need this too?

Thanks,
Julien Usson

Closes #11952

@oliviertassinari
Copy link
Member

oliviertassinari commented Oct 2, 2018

@JulienUsson Oh, I'm sorry for not being clear. I was suggesting using the recompose toRenderProps utility method to solves this problem, as well as adding a demo in the documentation. What do you think about it? We already have some demo with this approach: https://github.com/mui-org/material-ui/search?utf8=%E2%9C%93&q=toRenderProps&type=

@@ -35,10 +36,14 @@ Sometimes, using CSS isn't enough.
You might want to change the React rendering tree based on the breakpoint value, in JavaScript.
We provide a `withWidth()` higher-order component for this use case.

In the following demo, we change the rendered DOM element (*em*, <u>u</u>, ~~del~~ & span) based on the screen width.
In the following demo, we change the rendered DOM element (_em_, <u>u</u>, ~~del~~ & span) based on the screen width.
Copy link
Member

Choose a reason for hiding this comment

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

Can we keep the old convention? I think that it's better to reduce the entropy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, sorry, i haven't seen that vscode autoformat it.

@oliviertassinari oliviertassinari changed the title #11952 rename props in withWidth [withWidth] Document the render prop Oct 2, 2018
@oliviertassinari oliviertassinari added docs Improvements or additions to the documentation PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI labels Oct 2, 2018
@JulienUsson
Copy link
Contributor Author

I was suggesting using the recompose toRenderProps utility method to solves this problem, as well as adding a demo in the documentation. What do you think about it?

Yes, nice idea :) i will update my PR soon

@JulienUsson JulienUsson force-pushed the feature/with-width-rename-props branch 3 times, most recently from b9bd92d to aeee392 Compare October 6, 2018 14:23
@oliviertassinari oliviertassinari removed the PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI label Oct 10, 2018
@oliviertassinari oliviertassinari force-pushed the feature/with-width-rename-props branch from aeee392 to 2a5ba0d Compare October 10, 2018 20:23
@oliviertassinari oliviertassinari merged commit a34a2a5 into mui:master Oct 10, 2018
@oliviertassinari
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants