-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Can we abstract away the idea of working with regions... #2148
Comments
It should be fairly easy to move away from regions. To start, we would first need to provide all of the functionality that regions provide on the view itself. Once we make this available, I'm confident that people will find it preferable to work with. From there, we can add migration guides / potentially automate some of it, let it simmer for a few months, then finally remove the old convention for working with regions. I'll cover two big reasons why people interact with views directly:
This can be solved by making the region function as a placeholder. When a view is shown in a region, the default behavior should be that the region's element is plucked out of the DOM and replaced with the view's element. There are a slew of other benefits we get from this, as well. Minionette and Angular implement a placeholder-swap-and-replace algorithm, so we can look at those for inspiration. LayoutManager solves the wrapping problem in another way: it makes the view's elements optional. @tbranyen, correct me if I'm wrong, but you don't swap the view in for the matched element that you're showing? In other words, after I've called If the answer is yes, then I prefer swapping out the region's element vs. making the view's element optional. Both cases solve for the unnecessary wrapper in a large number of use cases. Another concern are dynamic regions. This is easily solved by letting folks quickly instantiate regions with selectors (or maybe even elements). When the UI updates, get a reference to the newly-added element and show a view in it. And that's that. Dynamic regions. Creating dynamic regions in a layoutview is pretty clunky right now.
The next step would be to remove all events from regions, and push these up to the view itself. Instead of Lastly, we would remove all API methods that directly return the region instance. You would always go through the view's API. Some other benefits of swappable regions are that we can get rid of the CompositeView class. The only reason CompositeView exists (ignoring the terrible recursive behavior) is that you need it to show tables. If we swap out the region's element, you can get the CompositeView effect with just a Marionette.LayoutView (or, in this system, the plain old Marionette.View). Minionette manages to pull off a CompositeView by allowing the CollectionView to have a template. I haven't looked at the API to see how that works, but it's worth looking into, I think. I know a number of people treat regions as quasi-view objects, listening to their events, overriding them, and so on, but I think that this is probably something we should move away from. We can accomplish the exact same things without this extra concept, with fewer code and a cleaner, more succinct API. I'm strongly in favor of us moving in the direction of making regions less of a 'feature' of Marionette. It'll be tough, but in the end, I think we would all be happy with the change, and Marionette would be a far cleaner library. |
mind expanding on this |
No time now, but I'll be working on it sometime soon-ish. |
ok the only reason I ask is because that is the hardest problem with a regions "rewrite". By that i mean async destroy of regions and their contents. |
@samccone for my own projects I've create an AnimatedRegion, which basically add some class on the view el to animate the view inside the region. My objective are quite simple:
It's still a work in progress while being used in some projects so expect some changes.
As I've already said before, this is still a working in progress, I would set beta, as I already use this in production, or next-to-be production code. Expect a few changes in the coming days. |
@jmeas kicked off an interesting discussion in #2145 w/r/t de-emphasizing regions.
@paulfalgout, @tbranyen weighed in with good points as well.
The text was updated successfully, but these errors were encountered: