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

appendChild null when using modals from ember-engines #175

Closed
tylerturdenpants opened this issue Dec 6, 2016 · 16 comments
Closed

appendChild null when using modals from ember-engines #175

tylerturdenpants opened this issue Dec 6, 2016 · 16 comments

Comments

@tylerturdenpants
Copy link

When using modals from an engine, the engine routes produces this error if {{bs-modal}} is used:

error-handler.js:436 TypeError: Cannot read property 'appendChild' of null
    at appendContainerElement (modals-container.js:413)
    at Object.initialize (modals-container.js:422)
    at ember.debug.js:4564
    at Vertices.each (ember.debug.js:2630)
    at Vertices.topsort (ember.debug.js:2597)
    at DAG.topsort (ember.debug.js:2542)
    at Class._runInitializer (ember.debug.js:4592)
    at Class.runInitializers (ember.debug.js:4553)
    at Class.ensureInitializers (ember.debug.js:4499)
    at Class.buildInstance (ember.debug.js:4513)

The host app has no issues. Ember 2.10, Ember-engines 0.4.0

@simonihmig
Copy link
Contributor

I have not worked with engines yet, unfortunately.

The null error comes from ember-bootstrap's initializer that created a <div id="ember-bootstrap-modal-container"> to put the modals into. In this case application.rootElement (http://emberjs.com/api/classes/Ember.Application.html#property_rootElement) seems to be null.

Maybe this is normal when using it in an engine, I can't tell. You could try to disable the initializer by setting insertEmberWormholeElementToDom to false:

let app = new EmberApp({
        'ember-bootstrap': {
            insertEmberWormholeElementToDom: false
        }
    });

Then you would have to manually add a div like shown above at the highest DOM level, so probably into the application template. Maybe this would solve it?

@tylerturdenpants
Copy link
Author

Guarding against the null seems to be what works:

  if( application && application.rootElement ){
  	appendContainerElement(application.rootElement, modalContainerElId);
  }

should I submit a PR?

@simonihmig
Copy link
Contributor

Would be welcome!

@tylerturdenpants
Copy link
Author

So far the workaround above only helps with the null issue. Once the view displays, ember-wormhole does not shuttle the modal to the container. I wonder how I'm going to open that issue... ;-)

@simonihmig
Copy link
Contributor

Do you have a <div id="ember-bootstrap-modal-container"> somewhere? Maybe in the application template of the host app?

@simonihmig
Copy link
Contributor

The initializer's job is to create it, but as this is failing in your case, it should be added somewhere, otherwise the modals will be inserted into the DOM where the component is defined, which is in general not the best place (think of a parent having `overflow: hidden')

@tylerturdenpants
Copy link
Author

Yes I do have the div. All of the host app's modals work. I can inspect the DOM and see the container with all my other modals. However, the modal that comes from the engine will render in place (like you mentioned above). This lead me to believe that the issue may reside in ember-wormhole

@simonihmig
Copy link
Contributor

This might be the case. It looks up the wormhole destination element here: https://github.com/yapplabs/ember-wormhole/blob/master/addon/components/ember-wormhole.js#L28. Maybe it somehow cannot "see" the div of the host app, just a sub tree of the DOM? (just guessing, not sure how engines work in detail)

But you can try with a simple single wormhole:

{{#ember-wormhole to="ember-bootstrap-modal-container"}}
    Hello world!
{{/ember-wormhole}}

If that does not work, the underlying problem could indeed be in ember-wormhole itself!

@tylerturdenpants
Copy link
Author

Drilling down even more today it seems that

{{#ember-wormhole to="ember-bootstrap-modal-container"}}
    Hello world!
{{/ember-wormhole}}

works just about everywhere (route templates, components) but as soon as I use {{bs-modal}}, It just displays in place and ignores the wormhole in the {{bs-modal}} template. Any Ideas? I think I will need to head over to ember-wormhole and add an issue there. Hopefully I cant make some headway.

@simonihmig
Copy link
Contributor

Hm, strange...

The modal checks for the existence of the container element, if that is not found, falls back to "renderInPlace". This is done in a computed property here: https://github.com/kaliber5/ember-bootstrap/blob/master/addon/components/bs-modal.js#L359-L361

It might be the case that when evaluating that property for the first time, that the container does not exist yet, and afterwards the property is cached as every CP, so does not reevaluate...

Does this make any difference:

{{#bs-model _renderInPlace=false}}...{{/bs-modal}}

@tylerturdenpants
Copy link
Author

tylerturdenpants commented Dec 9, 2016

Nada. :-(. After a bit more hacking, I finally got the stack trace to complain that the background element isn't there. Further assertions and errors led me to this error free usage

{{#bs-modal title="Simple Dialog" size="sm" fade=false open=false _renderInPlace=false}}
  Hi there
{{/bs-modal}}

but still it renders in plain sight. I have a feeling that this is a compound issue that includes glimmer 2, engines, and ember-wormhole. I'm fairly clueless on how to attack this any further.

@tylerturdenpants
Copy link
Author

@simonihmig after many days of more hacking it looks as though you have to import the layout

 import layout from '../templates/components/bs-modal';

export default Ember.Component.extend({
	layout,
...

to get the engine to wormhole the modal. I'm not too sure why the hbs for the modal lives in app while others live in addon. Could you explain?

@simonihmig
Copy link
Contributor

Hm, interesting, thanks for investigating!

There is no real reason other than putting the templates into app was the default of ember-cli in the early days when those components were created, while the newer ones use the different setup in addon (+ importing the layout).

I have refactored all of this already for the upcoming 1.0 release (see #158). But I am hesitant to change this for the current version because I am unsure if that could be a breaking change for current users which maybe rely on just overriding a compotent template by putting it into their app/templates folder.

@tylerturdenpants
Copy link
Author

No worries! I have forked this addon and patched what was needed to stay compatible with my app. I'd be happy to test drive the 1.0 release and see if meets my needs. Let me know if you are interested in any PR's from my fork but otherwise I think we can close the issue.

@simonihmig
Copy link
Contributor

OK. So by explicitly importing the layout into bs-modal the modals were finally running without any further issues in an engine?

If that is all it took, then I would not need a PR as this has already been done for the 1.0 branch as I said. But thanks anyway!

@tylerturdenpants
Copy link
Author

Ok! Btw the initializer is still an issue and the guard I mention earlier in the thread is viable but it has the issue of not being able to get the app context from the engine. I may look into this and submit real fix once I find one. Thank you!

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

No branches or pull requests

2 participants