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

Move View specific functionality to view #2842

Merged
merged 1 commit into from Dec 4, 2015

Conversation

paulfalgout
Copy link
Member

Related to #2841

Seems like there are some very View specific things that are in the shared to support CompositeView This moves all of that to the View and then to avoid further duplication, mixes in the methods from View into CompositeView

@jasonLaster
Copy link
Member

👍 I like two things about this:
(1) ViewMixin becomes simpler
(2) View becomes easier to reason about

I actually expect, that slowly going through some of these remaining shared methods will help us learn something about Marionette. As a sidenote, I was going to submit the massive PR w/ mixins, but I prefer this approach where we pluck a couple researched methods at a time.

And then mix it in to the CompositeView

Erring on the side of best View organization and less duplication, without breaking the deprecated CompositeView
@jasonLaster
Copy link
Member

obvious win.

jasonLaster added a commit that referenced this pull request Dec 4, 2015
Move View specific functionality to view
@jasonLaster jasonLaster merged commit 0edef86 into marionettejs:next Dec 4, 2015
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 this pull request may close these issues.

None yet

2 participants