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

WIP: Add auto discover of region element #3530

Closed
wants to merge 2 commits into from

Conversation

JSteunou
Copy link
Contributor

@JSteunou JSteunou commented Nov 8, 2017

Proposed changes

  • add auto discover of region in view

Link to the issue: #3417 #3389 #3320 and maybe #1972 ?

@JSteunou JSteunou added this to the v4.0 milestone Nov 8, 2017
@JSteunou JSteunou changed the title Add auto discover of region element WIP: Add auto discover of region element Nov 8, 2017
@JSteunou
Copy link
Contributor Author

JSteunou commented Nov 8, 2017

Some points I did not resolved around how far we want to go on this. I left some open doors for the user so he can

  1. use it's own selector: I set data-region by default but it could be data-mr-region
  2. use it's own logic on selection: I set a default data-region="${name}" but I'm currently using data-region="${_.kebabCase(name)}"
  3. add it's own region options: should Marionette add some feature around it, like data-region-replace => replaceElement: true or be very light and unopinionated? I started with light.
  4. should we raise an error when the element is not found?
  5. ...

@paulfalgout
Copy link
Member

so I do like this in theory, but I'd be interest on waiting until the core of region can be readdressed, or at least until we know how to readdress it.

I think some more code examples would help understand the feature fully as well.

@JSteunou
Copy link
Contributor Author

JSteunou commented Nov 9, 2017

So more v4.x or v5?

@paulfalgout
Copy link
Member

Yep I think v4.x - v5 should be focused on region like v3.x - 4 was on collectionView

@paulfalgout paulfalgout modified the milestones: v4.0, v4.x Nov 11, 2017
@taburetkin
Copy link
Contributor

taburetkin commented May 7, 2018

i don't know if it helps
but personally i have this head pain with regions

  1. i am always have to define template for container view
    f.e.
<div class='actions-region'></div>
<div class='entries-region'></div>
<div class='something-region'></div>
  1. i am always have to define regions in a container view definition
    f.e.
regions:{
   'actions':'.actions-region',
   'entries':{el:'.entries-region', replaceElement:true},
   'something':'.something-region'
}
  1. i am always have to provide onRender logic in every container view
    wich is in 99% of cases is something like this
let View = getActionsViewClass();
let options = getActionsViewOptions();
let view = new View(options);
this.showChildView('actions',view);
  1. and if there are few nested views in a container its looks pretty ugly

so, i ended with one mixin for a View and one helper

now my container view looks like

let Container = BaseContainerView.extend({
   template:_.template('')
   initialize(){
      this.createNested({
         name:'actions',
         viewClass: ActionsView,
         viewOptions: () => somehowBuildOptions(), //optional,
         regionTemplate:{ replaceElement: true }, //if i do not provide any selector information region will be created automaticaly
      })
   },
   onRender(){
       this.showAllNested();
   }
})

so, i do not care about container`s template any more
and i do not need to repeat nested views logic

@paulfalgout
Copy link
Member

Looks at regions on https://github.com/marionettejs/marionette

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.

3 participants