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

Custom modals don't have destinationElementId injected #22

Closed
rlivsey opened this issue Apr 27, 2015 · 6 comments · Fixed by #28
Closed

Custom modals don't have destinationElementId injected #22

rlivsey opened this issue Apr 27, 2015 · 6 comments · Fixed by #28

Comments

@rlivsey
Copy link
Contributor

rlivsey commented Apr 27, 2015

In our app we have some modals which are "full screen" and have some custom behaviour/settings (no overlay, different CSS etc…).

To accomplish this I made a new component which extends from ModalDialogComponent add adds in the custom behaviour so in my templates I can now do:

<full-screen-modal>
    modal contents here
</full-screen-modal>

This breaks because the wormhole's destination element ID is only injected into the 'modal-dialog' component.

This is easily worked around by adding another injection:

export default {
  after: 'add-modals-container',
  name: 'custom-modals',
  initialize(_, application) {
    application.inject('component:full-screen-modal',
                       'destinationElementId',
                       'config:modals-container-id');
  }
};

However, ideally we wouldn't have to do this.

@lukemelia
Copy link
Contributor

Interesting. We could get around this by creating a service to hold the default destinationElementId that is accessed via Ember.inject.service, so that subclasses will have access to it. Will noodle on this and open to other folks' feedback.

@mike-north
Copy link
Contributor

I found the same problem, and had to resort to the same solution https://github.com/sgasser/ember-cli-materialize/blob/master/app/initializers/add-modals-container.js#L12-L14

@sandstrom
Copy link
Contributor

Same issue here, same workaround. Ember.inject.service sounds like a good idea!

@lukemelia
Copy link
Contributor

OK, sounds like a plan. Open to a PR if someone wants to take this on.

@rlivsey
Copy link
Contributor Author

rlivsey commented May 5, 2015

@lukemelia I'm doing this in a pop-over addon for our app, using a service which just has one property holding the destinationElementId, that property is set from the initializer and then used in the component.

If that looks good, I can do the same here and will open a pull request later today.

@lukemelia
Copy link
Contributor

@rlivsey Sounds good to me. Thanks. Please update the docs with your PR, as I think this approach is a little more confusing, but clearly necessary given the feedback.

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

Successfully merging a pull request may close this issue.

4 participants