-
Notifications
You must be signed in to change notification settings - Fork 24
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
Update to Material-UI v4 #80
base: master
Are you sure you want to change the base?
Conversation
mtnori
commented
May 25, 2019
- Update to Material-UI v4
Can this version be pushed pls? :) |
Sorry I just haven't had time to review this yet. I did think it was going
to take a little bit more than bumping packages though and knew I didn't
have time to dig in yet.
…On Thu, May 30, 2019, 4:58 PM Chris Van Emmerik ***@***.***> wrote:
@techniq <https://github.com/techniq> @mtnori <https://github.com/mtnori>
This is close to being ready. I did notice 1 JS warning on the 'with
custom adornments' example. Once that is fixed (and also check all the
other examples to ensure no errors), I think this is ready.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#80?email_source=notifications&email_token=AABLKRCZYD3E7YNQSAQPM2DPYA5XDA5CNFSM4HPT3U72YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWTQAPY#issuecomment-497483839>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABLKRGTTNYVCIDWJOXWBUTPYA5XDANCNFSM4HPT3U7Q>
.
|
I think bumbping up react and material-ui looks good. It works for me with this package.json: { Hope that helps!! |
@Domino987 I am still getting JS warnings on the following demo examples:
|
Any progress on this? |
Friendly ping :) |
If someone could submit a PR that doesn't have JS errors or warnings on the examples, we can merge this. I spent some time on this initially but hit a dead end and couldn't resolve all of the warnings. Right now I don't have a whole lot of time to spend on this. @techniq We could merge with the warnings, but I don't think that is a good idea? |
@cvanem I wouldn't want to merge with warnings. If someone sends a PR without errors, I'll definitely merge and cut a release. |
I created #83 to update to Material-UI to v4 and fixed all of the warnings / errors except 1. |
@BenDiuguid I took a quick look at your PR but it looks like there are still numerous JS errors. If you launch the storybook and click through each example you can see the different JS errors in the console. The examples that have errors are outlined above. Those are what need to be resolved still. |
@cvanem Ahh I wasn't sure which JS errors / warnings you were referring to. To clarify also the storybook I may be busy this weekend, but I'll get around to this soon, and IIRC these issues are mostly related to PropTypes validation. |
PR #83 Is ready to be reviewed 👍 |