techniq / mui-downshift

Thin layer over paypal's downshift to use Material UI visual components
https://techniq.github.io/mui-downshift/
MIT License
92 stars 24 forks source link

Update to Material-UI v4 #80

Open mtnori opened 5 years ago

mtnori commented 5 years ago
Domino987 commented 5 years ago

Can this version be pushed pls? :)

cvanem commented 5 years ago

@techniq @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.

techniq commented 5 years ago

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 notifications@github.com 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 https://github.com/techniq/mui-downshift/pull/80?email_source=notifications&email_token=AABLKRCZYD3E7YNQSAQPM2DPYA5XDA5CNFSM4HPT3U72YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWTQAPY#issuecomment-497483839, or mute the thread https://github.com/notifications/unsubscribe-auth/AABLKRGTTNYVCIDWJOXWBUTPYA5XDANCNFSM4HPT3U7Q .

Domino987 commented 5 years ago

I think bumbping up react and material-ui looks good.

It works for me with this package.json:

{ "name": "mui-downshift", "version": "1.4.1", "author": "Sean Lynch techniq35@gmail.com", "license": "MIT", "repository": "techniq/mui-downshift", "files": [ "dist" ], "main": "dist/index.js", "keywords": [ "react", "downshift", "material-ui" ], "peerDependencies": { "@material-ui/core": "^4.0.1", "@material-ui/icons": "^4.0.1", "react": "^16.8.6", "react-dom": "^16.8.6" }, "dependencies": { "classnames": "^2.2.6", "downshift": "^2.2.0", "prop-types": "^15.7.2", "react-virtualized": "^9.20.1" }, "devDependencies": { "@material-ui/core": "^4.0.1", "@material-ui/icons": "^4.0.1", "@storybook/addon-actions": "^3.4.10", "@storybook/react": "^3.4.10", "babel-cli": "^6.26.0", "babel-eslint": "^8.2.6", "babel-plugin-transform-class-properties": "^6.24.1", "babel-plugin-transform-object-rest-spread": "^6.26.0", "babel-plugin-transform-react-jsx": "^6.24.1", "babel-preset-env": "^1.6.0", "eslint": "^5.4.0", "eslint-config-prettier": "^2.9.0", "eslint-plugin-babel": "^5.1.0", "eslint-plugin-import": "^2.14.0", "eslint-plugin-prettier": "^2.6.2", "eslint-plugin-react": "^7.11.1", "gh-pages": "^1.2.0", "prettier": "^1.14.2", "react": "^16.8.6", "react-dom": "^16.8.6" }, "scripts": { "build": "NODE_ENV=production babel src -d dist", "storybook": "start-storybook -p 9009 -c stories", "build-docs": "cd stories && yarn install && cd .. && build-storybook -c stories -o docs", "deploy-docs": "gh-pages -d docs", "preversion": "yarn build", "postpublish": "yarn build-docs && yarn deploy-docs" } }

Hope that helps!!

cvanem commented 5 years ago

@Domino987 I am still getting JS warnings on the following demo examples:

MaxLeiter commented 5 years ago

Any progress on this?

BenDiuguid commented 5 years ago

Friendly ping :)

cvanem commented 5 years ago

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?

techniq commented 5 years ago

@cvanem I wouldn't want to merge with warnings. If someone sends a PR without errors, I'll definitely merge and cut a release.

BenDiuguid commented 5 years ago

I created #83 to update to Material-UI to v4 and fixed all of the warnings / errors except 1.

cvanem commented 5 years ago

@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.

BenDiuguid commented 5 years ago

@cvanem Ahh I wasn't sure which JS errors / warnings you were referring to. To clarify also the storybook stories/ are the only examples right?

I may be busy this weekend, but I'll get around to this soon, and IIRC these issues are mostly related to PropTypes validation.

BenDiuguid commented 5 years ago

PR #83 Is ready to be reviewed 👍