-
Notifications
You must be signed in to change notification settings - Fork 3.4k
docs(dialog): correct and add back resolve property #11164
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here (e.g. What to do if you already signed the CLAIndividual signers
Corporate signers
|
I signed it! |
CLAs look good, thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for sending this PR!
I think that one minor change to the type will help make this more clear.
src/components/dialog/dialog.js
Outdated
@@ -536,6 +536,8 @@ function MdDialogDirective($$rAF, $mdTheming, $mdDialog) { | |||
* `three` into the controller, with the value 3. If `bindToController` is true, they will be | |||
* copied to the controller instead. | |||
* - `bindToController` - `bool`: bind the locals to the controller, instead of passing them in. | |||
* - `resolve` - `{object=}`: Similar to locals, except it takes as values functions that return promises, and the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's change {object=}
to {function=}
to match the changes to the text.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops. Correcting it right away.
@Splaktar This was just a docs edit. Ideally build should not fail. Read from other PRs that there was a Travis CI outage last night affecting the tests. |
Yep, don't worry about that test failure. Can you please squash your commits and update the commit message to have |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM once squashed and commit message updated.
bbb50c0
to
dad30ab
Compare
👍 Squashed and commit message updated. Thank you for bearing with me. |
Corrected resolve property description Closes angular#11159
No problem at all. I understand that we have some requirements and elements of our process that take some time to learn! Thank you for keeping with it and making the requested updates! |
Corrected resolve property description Closes angular#11159
Corrected resolve property description Closes #11159
PR Checklist
Please check that your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Changes docs. Corrects and adds back resolve property doc in the $mdDialog.show() parameters list.
What is the current behavior?
Currently the resolve property is not listed or described in the $mdDialog.show(optionsOrPreset) parameters documentation. Refer to issue #7400. But as suggested in comments, slight documentation correction was required for the property.
Issue Number:
#11159
What is the new behavior?
The documentation for the resolve property has been updated and added back.
Does this PR introduce a breaking change?
Other information