Clean up item content observers on segmented views #791

Merged
merged 2 commits into from Jun 11, 2012

Conversation

Projects
None yet
2 participants
Member

tim-evans commented Jun 1, 2012

When updating the segment content items, segmented views did not remove observers from the previous content. This caused old observers to hang around after the items should be removed from the segments. This occurred both when extra segments were destroyed as well as when content was shifted around within existing tabs. And, because 'removed' items remembered their old indices when the observers fired, they would sometimes re-insert themselves into their old segment.

To fix this, we remove observers from SC.Object items when we delete extra segments, and when we update the content we check to see if each old item will remain in the items list. If they will be removed, we clean up the observers before we update the segment.

@tim-evans Will Mitchell SegmentedView - Remove observers from items when they are removed fro…
…m the view, prevents a case where a removed item calls itemContentDidChange and puts itself back in a segment
bb9ac74

@publickeating publickeating commented on an outdated diff Jun 5, 2012

frameworks/desktop/tests/views/segmented/observers.js
+ })]
+ });
+ pane.append(); // make sure there is a layer...
+ SC.RunLoop.end();
+
+ view = pane.childViews[0];
+ },
+
+ teardown: function() {
+ pane.remove();
+ pane = view = null ;
+ }
+});
+
+test("Check that observers are removed properly", function() {
+ debugger;
@publickeating

publickeating Jun 5, 2012

Owner

Whoops. debugger.

Owner

publickeating commented Jun 5, 2012

Just so I'm clear, the problem is that SC.Objects removed from the list of items, may not necessarily be destroyed, so we have to be sure to clear the observer? That makes total sense to me, I'll merge if you can get rid of that debugger statement.

Thanks @tim-evans

Member

tim-evans commented Jun 6, 2012

That's exactly correct. And I'll get on removing that debugger statement!

@tim-evans tim-evans added a commit that referenced this pull request Jun 11, 2012

@tim-evans tim-evans Merge pull request #791 from sproutcore/team/tim-evans/segmented-view…
…-leak

Clean up item content observers on segmented views
2aecc88

@tim-evans tim-evans merged commit 2aecc88 into master Jun 11, 2012

Member

tim-evans commented Jun 11, 2012

It's been a week, so I'm merging this in...

nicolasbadia deleted the team/tim-evans/segmented-view-leak branch Jan 23, 2015

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