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

[Regression] disconnectOutlet can't reach across routes #10606

Closed
ghost opened this issue Mar 11, 2015 · 6 comments · Fixed by #10683
Closed

[Regression] disconnectOutlet can't reach across routes #10606

ghost opened this issue Mar 11, 2015 · 6 comments · Fixed by #10683
Labels
Milestone

Comments

@ghost
Copy link

ghost commented Mar 11, 2015

Disconnecting an outlet used to work even from a sub route. Now the element doesn't get removed from the DOM.

Modal doesn't get removed In Ember 1.11.0-beta.5: http://emberjs.jsbin.com/dofohetuhu/5/edit
Modal gets removed in Ember 1.11.0-beta.1: http://emberjs.jsbin.com/dofohetuhu/3/edit

Seems to have started from beta.2, after this change ba1d1a6

@ghost ghost changed the title disconnectOutlet semantics changed 1.11.0-beta.5 disconnectOutlet behaviour changed in 1.11.0-beta.5 Mar 11, 2015
@rwjblue rwjblue added this to the 1.11.0 milestone Mar 11, 2015
@rwjblue rwjblue added the Bug label Mar 11, 2015
@mutewinter
Copy link
Contributor

👍 also experiencing this on 1.11.0-beta.5

@mutewinter
Copy link
Contributor

My experience is that a controller triggering the disconnect via a triggerAction that hits the application controller fails. My temporary fix was to create a bugFixDisconnectOutlet function that calls disconnectOutlet twice with the same arguments.

@mutewinter
Copy link
Contributor

My experience is that a controller triggering the disconnect via a triggerAction that hits the application controller fails. My temporary fix was to create a bugFixDisconnectOutlet function that calls disconnectOutlet twice with the same arguments.

Actually, it turns out that I'm needing to call disconnectOutlet for every time the outlet was rendered.

http://emberjs.jsbin.com/xibuvaduzu/1/edit?html,js,output

Test 1

  1. Click Open Layer
  2. Click Close
  3. Modal closes fine

Test 2

  1. Click Open Layer
  2. Click Open Layer Again
  3. Click Close
  4. Modal does not close
  5. Click Close
  6. Modal closes

Close needs to be clicked N number of times, where N is the number of times the outlet was rendered into.

@mutewinter
Copy link
Contributor

And here's my temporary fix:

  bugFixRender: (view, options) ->
    @disconnectOutlet(
      outlet: options.outlet
      parentView: options.into
    )
    @render(view, options)

@ef4
Copy link
Contributor

ef4 commented Mar 20, 2015

PR #10683 addresses @mutewinter 's comment about stacking multiple calls to render.

The original issue reported by @Soliah is a regression and still open.

@ef4 ef4 changed the title disconnectOutlet behaviour changed in 1.11.0-beta.5 [Regression] disconnectOutlet can't reach across routes Mar 20, 2015
ef4 added a commit to ef4/ember.js that referenced this issue Mar 20, 2015
Closes emberjs#10478 by changing the way we account for routes that don't do a
default `render()`.

Fixes issue in comments of emberjs#10606 to make us tolerant of multiple renders on top of each other.

Close emberjs#10658 by respecting a non-default template rendered into a
main outlet as our own template.

Close emberjs#10606 by running disconnectOutlet against all active routes, not
just the current one. (*shudder*)
@ef4
Copy link
Contributor

ef4 commented Mar 20, 2015

Now I've extended PR #10683 to also close the original issue.

ef4 added a commit that referenced this issue Mar 26, 2015
Closes #10478 by changing the way we account for routes that don't do a
default `render()`.

Fixes issue in comments of #10606 to make us tolerant of multiple renders on top of each other.

Close #10658 by respecting a non-default template rendered into a
main outlet as our own template.

Close #10606 by running disconnectOutlet against all active routes, not
just the current one. (*shudder*)

(cherry picked from commit a994491)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants