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

Modal, prevent null action error for onclose #60

Open
crodriguez1a opened this issue Jun 1, 2017 · 2 comments
Open

Modal, prevent null action error for onclose #60

crodriguez1a opened this issue Jun 1, 2017 · 2 comments
Assignees

Comments

@crodriguez1a
Copy link
Member

crodriguez1a commented Jun 1, 2017

In the demo, we provide an onclose action conditionally, and a null value can be passed as a closure action, which is not allowed in 2.13.

Can either update the demo here to demonstrate the following:

{{bulma-modal show=showModal onclose=(if showModal (toggle 'showModal' this) (action 'noOp'))}}

or provide a noOp as the default onclose value

@jbailey4
Copy link
Member

@crodriguez1a Seems like conditionally setting the onclose action is a side-effect of using the show attr to show/hide the modal. What if we slightly changed the implementation, where we instruct users to use the if helper ->

{{if showModal}}
  {{bulma-modal onclose=(toggle 'showModal' this)}}
{{/if}}

Some advantages I see with this are:

  • {{bulma-modal}} could properly take advantage of component lifecyle hooks (i.e. didInsertElement and willDestroyElement)
  • able to remove the conditional onclose action
  • the {{bulma-modal}} component is only in the DOM when needed
  • more like other implementations (e.g. ember-modal-dialog)

@alexdiliberto
Copy link
Contributor

@jbailey4's suggestion seems like the idiomatic approach to me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants