Add onClose attribute to polaris-popover #131
Conversation
addon/components/polaris-popover.js
Outdated
| @@ -87,10 +87,9 @@ export default Component.extend({ | |||
| * | |||
| * @property onClose | |||
| * @type {function(source: React.ReactElement)} | |||
There was a problem hiding this comment.
lol, we should probably update this to just be function 😉
| @@ -1,6 +1,7 @@ | |||
| {{#basic-dropdown | |||
| verticalPosition=verticalPosition | |||
| onOpen=(action "onOpen") | |||
| onClose=onClose | |||
There was a problem hiding this comment.
Wrap this in an action helper as well to ensure context is always correctly maintained (it's what @vladucu would want 😉)
There was a problem hiding this comment.
yes, please...it's also much easier to read 😄
| @@ -6,7 +6,7 @@ | |||
| component](https://polaris.shopify.com/components/overlays/popover). This component uses [`ember-basic-dropdown`](https://github.com/cibernox/ember-basic-dropdown) to implement popover functionality. Note that the usage is slightly different from the React implementation so please pay attention to the examples below. | |||
|
|
|||
|
|
|||
| **NOTE:** _the `active`, `activatorWrapper`, `preventAutofocus` and `onClose` properties are currently unimplemented._ | |||
| **NOTE:** _the `active`, `activatorWrapper`, and `preventAutofocus` properties are currently unimplemented._ | |||
There was a problem hiding this comment.
Nice job remembering this 👍 Can you maybe add an example of using onClose here as well? Either work it into one of the existing examples or add a new one.
|
|
||
| // open the popover | ||
| click(activatorSelector); | ||
|
|
There was a problem hiding this comment.
Maybe check that the action hasn't run at this point just to be concrete about things? 🤷♂️
|
Also can you check what's up with Travis pls? Want a green light before merging this 👍 |
tomnez
force-pushed
the
polaris-popover-on-close
branch
from
1fb610f
to
9d7918d
Compare
|
@andrewpye updated, and thanks to the mighty @vladucu for the travis fix 👊 |
Adds the
onCloseproperty to the popover component