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

[Snackbar] Fix SnackbarOrigin TypeScript definition #12083

Merged
merged 4 commits into from
Jul 9, 2018

Conversation

kobelobster
Copy link
Contributor

This PR fixes #12072 by

  • Renaming all occurences of SnackBar to Snackbar
  • Making horizontal and vertical properties of SnackbarOrigin mandatory.

Please note, that this is a BC break, but I think we don't follow Semver, do we @oliviertassinari ?

Result of yarn test

yarn run v1.7.0
$ yarn lint && yarn flow && yarn typescript && yarn test:unit
$ eslint . --cache && echo "eslint: no lint errors"
(node:9743) [ESLINT_LEGACY_OBJECT_REST_SPREAD] DeprecationWarning: The 'parserOptions.ecmaFeatures.experimentalObjectRestSpread' option is deprecated. Use 'parserOptions.ecmaVersion' instead. (found in "node_modules/eslint-config-airbnb-base/index.js")
eslint: no lint errors
$ flow --show-all-errors
No errors!
$ tsc -p tsconfig.json
$ cross-env NODE_ENV=test mocha packages/material-ui/test/**/*.test.js packages/material-ui/src/{,**/}*.test.js docs/src/**/*.test.js


  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․

  1570 passing (4s)

Done in 15.53s.

@kobelobster
Copy link
Contributor Author

Hm, why does test_browser fail? I don't really get the error

Should not have any git not staged

because I pushed changes in snackbar.md. What's exactly meant by that?

@oliviertassinari
Copy link
Member

because I pushed changes in snackbar.md. What's exactly meant by that?

@tzfrs run yarn docs:api you should be good :)

@oliviertassinari oliviertassinari added bug 🐛 Something doesn't work typescript labels Jul 8, 2018
@oliviertassinari oliviertassinari changed the title Rename SnackBarOrigin to SnackbarOrigin and make SnackbarOrigin properties mandatory [Snackbar] Fix SnackbarOrigin TypeScript definition Jul 8, 2018
@oliviertassinari oliviertassinari self-assigned this Jul 8, 2018
@oliviertassinari oliviertassinari merged commit f1e82a8 into mui:master Jul 9, 2018
@oliviertassinari
Copy link
Member

@tzfrs It's a great first pull request on Material-UI 👌🏻. Thank you for giving it a shot!

@kobelobster
Copy link
Contributor Author

kobelobster commented Jul 9, 2018

@oliviertassinari I saw your commit, but it's not correct. I made the horizontal and vertical required and your commit fb4c8f1#diff-a34c3e44f550e3e16cf0c3e4c049a119R19 made the parameters in the doc optional again, however they MUST be defined as horizontal and vertical (no question mark), don't they?

@oliviertassinari
Copy link
Member

@tzfrs I have made nothing, I have run the tool that automatically generates the documentation. Thanks for raising, it's a broader issue. The Popover is also affected (anchorOrigin, transformOrigin)

@kobelobster
Copy link
Contributor Author

Ah okay, but how does the popover affect the snackbar documentation? Are we okay with having a "wrong" documentation for now?

@oliviertassinari
Copy link
Member

Well, this pull request is fixing the TypeScript definition. It's already a good step forward. Now, we need to fix the prop-types.

acroyear pushed a commit to acroyear/material-ui that referenced this pull request Jul 14, 2018
* Make SnackBarOrigin properties mandatory

* Make SnackBarOrigin properties mandatory

* Rename SnackBar to Snackbar

* yarn docs:api
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work typescript
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants