Skip to content

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 type: bug It doesn't behave as expected. 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

type: bug It doesn't behave as expected. typescript

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants