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

[CONTRIBUTING] Document starting the documentation sooner #23757

Closed
rohan-buechner opened this issue Nov 29, 2020 · 10 comments · Fixed by #23801
Closed

[CONTRIBUTING] Document starting the documentation sooner #23757

rohan-buechner opened this issue Nov 29, 2020 · 10 comments · Fixed by #23801
Labels
docs Improvements or additions to the documentation good first issue Great for first contributions. Enable to learn the contribution process.

Comments

@rohan-buechner
Copy link

I'm struggling to figure out how to get a dev instance of MUI running on my local machine.

Context:
It all started from this question I asked on StackOverflow.

So far I've run the yarn start from the documentation site, which does run the site... but when, for example I try to add a new sub section under pages > customisation > variants (this is new) . Hitting the url alway results in a 404.

  • [x ] I have searched the issues of this repository and believe that this is not a duplicate.

Summary 💡

My issue is 2 fold, I'd like to actually solve the issue below. (I'm happy to do the work myself, except I'm not sure how to get a dev MUI project running and all the pieces working together)

I'd want to get a proof of concept working where I can have a component use a custom variant & palette.
like this. The problem, custom palette values seem to be ignored when using a custom variant too

<Button variant="MyCompaniesVariant" color="focus" />
<Button variant="MyCompaniesVariant" color="muted" size="large" />

https://codesandbox.io/s/globalthemevariants-material-demo-forked-qxjb3?file=/eleMuiGlobals/variant-extensions.ts

Motivation 🔦

I'm trying to get a small enhancement into a local copy of MUI to see how / if this would be possible.

Question

I looked through the MUI code base, but I'm either a little blind, or stupid / both :P But I'm not able to figure out the dev workflow. What do I need to do to "get up and running".

@rohan-buechner rohan-buechner added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Nov 29, 2020
@oliviertassinari
Copy link
Member

oliviertassinari commented Nov 29, 2020

What do I need to do to "get up and running".°

You need one command yarn && yarn docs:dev. It should be covered in https://github.com/mui-org/material-ui/blob/next/CONTRIBUTING.md#trying-the-changes-on-the-documentation-site. I think that we should update the guide to have this among the first sections. There is too much content to scan to find it.

@oliviertassinari oliviertassinari added docs Improvements or additions to the documentation and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Nov 29, 2020
@mbrookes
Copy link
Member

mbrookes commented Nov 29, 2020

You need one command yarn && yarn docs:dev.

That's two commands. 😁

You need one command: yarn start (which does the same thing). But @rohan-buchner has already done that. Agree that we should move it higher. It came up on another issue last week.

Instead, the initial question seems to be about adding a new page to the docs (docs/pages/customisation/variants). @rohan-buchner you'll need to restart the docs after adding a new page js file, and add it to `docs/src/pages.js if you want to add it to the menu. For pure experimentation though, I'd just hack on on of the existing demos.

For the second part of the question, (custom color with custom variant), cc @mnajdova

@oliviertassinari
Copy link
Member

oliviertassinari commented Nov 29, 2020

@mbrookes Oh ok. So it was a question about Next.js. I think that we can apply the following diff to make it easier to start contributing:

diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index e0390b4002..2899ab0ace 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -11,6 +11,21 @@ Please read [the full text](/CODE_OF_CONDUCT.md) so that you can understand what

 There are [many ways](https://material-ui.com/getting-started/faq/#material-ui-is-awesome-how-can-i-support-the-project) to contribute to Material-UI, code contribution is one aspect of it. For instance, documentation improvements are as important as code changes.

+## Trying changes on the documentation site
+
+The documentation site is built with Material-UI and contains examples of all the components.
+This is a great place to experiment with your changes.
+
+To get started:
+
+```sh
+yarn start
+```
+
+You can now access the documentation site [locally](http://localhost:3000).
+Changes to the docs will hot reload the site.
+
 ## Your first Pull Request

 Working on your first Pull Request? You can learn how from this free video series:
@@ -180,25 +195,6 @@ on their a11y-tree membership makes no difference. The queries where this does
 make a difference explicitly include this check e.g. `getByRole('button', { hidden: false })` (see [byRole documentation](https://testing-library.com/docs/dom-testing-library/api-queries#byrole) for more information).
 To see if your test (`test:browser` or `test:unit`) behaves the same between CI and local environment, set the environment variable `CI` to `'true'`.

-### Trying the changes on the documentation site
-
-The documentation site is built with Material-UI and contains examples of all the components.
-This is a great place to experiment with your changes.
-
-To get started:
-
-```sh
-yarn
-yarn docs:dev
-```
-
-You can now access the documentation site [locally](http://localhost:3000).
-Changes to the docs will hot reload the site. If you make changes to TypeScript files
-in the docs run `yarn docs:typescript --watch` in a separate terminal.
-
-Where possible, please add tests for any changes you make.
-Tests can be run with `yarn test`.
-
 ### Updating the component API documentation

 The component API in the component `propTypes` and under `docs/pages/api-docs` is auto-generated from the JSDOC in the TypeScript declarations.

@rohan-buechner
Copy link
Author

@mbrookes you are correct. I have followed all those steps. But somehow my changes didnt reflect. I'll try this in a bit (I still have a dayjob unfortunately :P), but I think this is the crucial part.

you'll need to restart the docs after adding a new page js file

And yes, my ultimate want is to have the palette work with a custom variant (Sorry about the 2 fold ticket or question)

Thank you all for the great lib & support.

@hmaddisb
Copy link
Contributor

@oliviertassinari I could include the change you have suggested for this issue in the CONTRIBUTION.md update I am going to include in my PR for #23583 ? Since they both relate to documentation changes.

@mnajdova
Copy link
Member

@rohan-buchner the styles defined in the variants, win over the default styles, and you have this line in your variant's styles:

    background: colors.charcoal100,

That's why that color is set as background. You can add an additional override for the combinations of the new variant and color: primary, and add the primary color as a background. I've updated your codesandbox here (using simply blue and red colors for the primary and secondary colors):

https://codesandbox.io/s/globalthemevariants-material-demo-forked-9sttf?file=/ele-mui-button/rounded.tsx

The size works because you are not overriding anything regarding size in your variant styles.

@oliviertassinari
Copy link
Member

oliviertassinari commented Nov 30, 2020

It seems that the different concerns were addressed, (this issue bundle a lot of different topics). I have renamed it to focus on one actionable item: how to start the documentation as one of the first steps: #23757 (comment).

@hmaddisb we try to keep pull requests focusing on a single problem at a time. In this case, It's probably best to have two different pull requests as they can land independently.

@oliviertassinari oliviertassinari changed the title Quick start docs for getting a dev setup started [CONTRIBUTING] Document starting the documentation sooner Nov 30, 2020
@oliviertassinari oliviertassinari added the good first issue Great for first contributions. Enable to learn the contribution process. label Nov 30, 2020
@rohan-buechner
Copy link
Author

@mnajdova I got it! Thank you! (Still a little new to some of the more advanced MUI usages) But that but that updated sandbox you've created helped tremendously. Thank you so much.

@mbrookes
Copy link
Member

@rohan-buchner Is there a gap in the documentation? If so, how could we improve it?

@rohan-buechner
Copy link
Author

The documentation is actually good... its just the sheer volume of the docs & the size of the project.

I for example. Before asking, cloned the project, got it up and running and managed to add a new route as per your suggestion... But I just couldn't get the page to appear

Off the cuff I cant think of anything exactly, other than it being daunting/overwhelming when jumping in the first time, and the items below are just subjective and nit picky...

  • Potentially re-arranging this doc a little.

Explain how to get a person's dev environment up and running before you explain how to do a PR. (Just to potentially align with what someone might do in the real world)

  • But I'm just thinking what I'd do when a new dev joins our company's code base.

It might be worth it to have 1 sample in the doc explaining how to add/modify something to one of the existing components. Like a walk through. I do realise that in itself is a tall order. This link is perfect at addressing the above... Again, missed it 😭 . Point number 4

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 good first issue Great for first contributions. Enable to learn the contribution process.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants