Skip to content

Hook demos #15032

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

Closed
oliviertassinari opened this issue Mar 24, 2019 · 9 comments
Closed

Hook demos #15032

oliviertassinari opened this issue Mar 24, 2019 · 9 comments
Labels
docs Improvements or additions to the documentation

Comments

@oliviertassinari
Copy link
Member

oliviertassinari commented Mar 24, 2019

With #13497 we got a lot of demos using the hook API. We want to continue this effort with the goal to have all demos using the hook API, only. We want to encourage a single pattern for each problem, not many.

styles

People are wondering when to use withStyle, makeStyles or the Box component.
Most of the time, you should consider using the hook API.
There is one case where withStyles can be preferred over makeStyles. In the following case, using the hook API would create more boilerplate:

const CustomTableCell = withStyles(theme => ({
  head: {
    backgroundColor: theme.palette.common.black,
    color: theme.palette.common.white,
  },
  body: {
    fontSize: 14,
  },
}))(TableCell);

makeStyles has many advantages over withStyles, it requires less boilerplate, it's faster, it's easier to type, it's easier to move & delete.

If you want to make contributions to this repository, we would appreciate you helping us.

Getting started

TL;DR: Use #15031 as example workflows.

  1. Set up your fork (See 'getting started" in the contributing guide).
  2. Choose a demo you want to convert. For this example, we'll use: https://next.material-ui.com/demos/lists/#nested-list
  3. go to ./docs/src/pages/demos/lists/lists.md and find the section. You're looking for a {{ "demo": "some-path" }} block:
  {{"demo": "pages/demos/lists/NestedList.js"}}
                               ^^^ Name of the demo file in `./docs/src/pages/demos/lists/` 
  1. Make sure a TypeScript variant is available. If it's not, you should handle TypeScript demos #14897 first in a different pull request.
  2. Only migrate the TypeScript variant. The TypeScript variant of the demo (.tsx) is used to generate the JavaScript variant (.js).
  3. run docs:typescript:formatted to update the JavaScript version.
  4. Submit a PR

While working

If you're stuck at any point you can still submit the PR. A maintainer will take a look and provide guidance.

This migration task is highly repetitive, we should be able to largely automate it with the appropriate search & replace.

@oliviertassinari oliviertassinari added docs Improvements or additions to the documentation help wanted labels Mar 24, 2019
@oliviertassinari oliviertassinari pinned this issue Mar 24, 2019
@apanizo
Copy link
Contributor

apanizo commented Mar 25, 2019

I will gladly help on this, I want to get more comfortable with the monorepo, so this task seems to be perfect for me, but unfortunately, I do not have too much free time. I guess until the end of the week I won't be able to help (but I can not promise anything).

@oliviertassinari
Copy link
Member Author

@apanizo Don't worry, you can contribute at your own pace :)

@fzaninotto
Copy link
Contributor

Am I understanding correctly that the makeStyles API doesn't allow the styles to be overridden by the caller of the component the way withStyles did? If so, the fact that you use makeStyles internally is a major BC break, as it doesn't allow to override default styles anymore...

@oliviertassinari
Copy link
Member Author

Yes, it does. You need to provide the props as a first argument of useStyles.

@fzaninotto
Copy link
Contributor

Thanks for your answer. I'm not sure I understand which props you're talking about. And that makes me think that I couldn't find any documentation on overriding styles for a component styled with makeStyles...

@eps1lon
Copy link
Member

eps1lon commented Apr 18, 2019

function Component ({ classes }) {
   ...
}
export default withStyles(styles)(Component);

is equivalent to

const useStyles = makeStyles(styles);
function Component (props) {
   const classes = useStyles(props);
   ...
}
export default Component;

@eps1lon
Copy link
Member

eps1lon commented Apr 18, 2019

The override works the same for the user. The implementation just has to make sure it calls useStyles with props. This is IMO a better API since you now have full control over your CSS API (expose nothing, everything, only some classes etc).

@oliviertassinari
Copy link
Member Author

that I couldn't find any documentation on overriding styles for a component styled with makeStyles...

@fzaninotto Correct, I'm working on it in #15140.

@oliviertassinari
Copy link
Member Author

I'm closing, this is not important enough. We will take care of removing React.Component from the demos in #15231. As for the styles, it would make sense to migrate all the demos to either use the Box or styled-components, hopefully.

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

No branches or pull requests

4 participants