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

Raise an event when all views in a given selector are closed #454

Merged
merged 5 commits into from Nov 10, 2014
Merged

Raise an event when all views in a given selector are closed #454

merged 5 commits into from Nov 10, 2014

Conversation

gjlubbertsen
Copy link
Contributor

@tbranyen
Copy link
Owner

Please follow the existing code style and do not add your own. See:

https://github.com/tbranyen/backbone.layoutmanager/blob/master/CONTRIBUTING.md#code-style

@tbranyen
Copy link
Owner

Looks good otherwise 👍

@gjlubbertsen
Copy link
Contributor Author

ok. Will fix the same

// If the managers match, splice off this View.
if (view && view.__manager__ === manager) {
aSplice.call(parentViews, i, 1);
}
});
if (_.isEmpty(parentViews)){
manager.parent.trigger("removedLastViewForSelector",
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use 2 space soft tabs, please adjust.

// If the managers match, splice off this View.
if (view && view.__manager__ === manager) {
aSplice.call(parentViews, i, 1);
}
});
if (_.isEmpty(parentViews)) {
manager.parent.trigger("removedLastViewForSelector",
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would a better event name be empty?

manager.parent.trigger("empty");

//...

view.on("empty", function(selector) {

});

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it. Little less speaking but think it covers what happened.

@tbranyen
Copy link
Owner

Awesome, once those changes are done, I think it's good to go, but I'd like a second look from @STRML @SBoudrias @jugglinmike or @ssafejava.

@gjlubbertsen
Copy link
Contributor Author

Ok. Lets see what they think about it.

@gjlubbertsen
Copy link
Contributor Author

wrote initial documentation:
https://github.com/tbranyen/backbone.layoutmanager/wiki/Events

@jugglinmike
Copy link
Collaborator

@gjlubbertsen Might want to hold off on modifying the wiki at least until it lands. Or even better: until we release a new version.

@gjlubbertsen
Copy link
Contributor Author

Fine.

@gjlubbertsen
Copy link
Contributor Author

reverted it: list here for reference.

empty

If you have a layout with sub-views you might want to be informed when the last view in a selector is removed.
In that case the empty event is triggered. This allows your layout to hide unnecessary parts of the layout.

myLayout.on("empty", function(selector) {
  if (selector == "#rightpanel") {
    $("#rightcontainer").hide();
  }
});

@gjlubbertsen
Copy link
Contributor Author

When can we expect it to board the master ?

tbranyen added a commit that referenced this pull request Nov 10, 2014
Raise an event when all views in a given selector are closed
@tbranyen tbranyen merged commit cd0b268 into tbranyen:master Nov 10, 2014
@tbranyen
Copy link
Owner

Right now ;-)

@gjlubbertsen
Copy link
Contributor Author

Thanks!

When do we post the documentation changes?

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