MithrilJS / mithril.js

A JavaScript Framework for Building Brilliant Applications
https://mithril.js.org
MIT License
14.02k stars 925 forks source link

There's no way to use the shouldReplaceHistory flag with {config: m.route} #546

Closed farzher closed 8 years ago

farzher commented 9 years ago
m.route('/whatever', true)

The true flag here tells Mithril to replace the history, instead of push it But this option doesn't exist when using config to route

{config: m.route} // How do I replace?

Maybe someone has a clever idea of how to expose this option to the config version? I don't.

nordfjord commented 9 years ago

you could use an onclick handler like so:

{
  onclick: function(e){
    // so as to not actually activate the link
    e.preventDefault();
    m.route(e.target.href, true);
  }
}
farzher commented 9 years ago

@nordfjord I guess that's fine, since this is not a common thing to do.

Is there a way to make the default method of routing be replace? In my case, I need to do this for the whole app. So I'd rather use the idiomatic code, and just add something like m.route.replaceHistory = true I don't think I can do this without hacking mithril source

lhorie commented 9 years ago

Not exactly pretty, but for now, you could create a custom m.route-like config, e.g.

var replaceRoute = function(el, init, ctx, vdom) {
  if (!init) el.onclick = function(e) {
    e.preventDefault()
    m.route(vdom.attrs.href, true)
  }
}
farzher commented 9 years ago

@lhorie What's the point of the vdom stuff instead of just using el?

nordfjord commented 9 years ago

you could make the route function as such:

window.replaceRoute = function(route){
  return function(e){
     e.preventDefault();
     m.route(route, true); 
 }
}

and use it in your templates like this:

m('a', {
  href: href,
  onclick: replaceRoute(href)
}, 'replace route')
lhorie commented 9 years ago

@farzher you could use el.href too if you just care about grabbing the value. The snippet I posted is a bit oversimplified because it doesn't deal w/ middle clicks and stuff like that. The vdom happens to be easier to work with if you do handle those edge cases.

farzher commented 9 years ago

@lhorie Is adding an option to switch the default value of shouldReplaceHistory from false to true not something you want to do? i.e. m.route.replaceHistory = true Which would be used as the default here

My app is a chrome extension running in an iframe, and adding to the browser history is not something I ever want to do in this app.

lhorie commented 9 years ago

I'm not against adding it, and like parseQuerystring/buildQuerystring, it'd be beneficial for such a flag to be in core just because of the sheer number of annoying edge cases that you'll have to deal w/ if it isn't (the middle click stuff for example).

I'm just not really sure what's the best way to expose that flag in a config setting. A global, as you suggested, is a reasonable option, but ideally I'd also like to allow it on a per-config basis as well.

barneycarroll commented 9 years ago
{
    href    : uri,
    onclick : m.route.bind( this, uri, true )
}

?

nordfjord commented 9 years ago

@barneycarroll I believe that example wouldn't work since it's sans preventDefault, causing the browser to actually go to the href.

barneycarroll commented 9 years ago

config : m.route is designed to make links work like standard links within the context of Mithril routing, whereas the behaviour we're after is something that needs scripting anyway. preventDefault is an easily solvable problem:

{
  href : uri,
  onclick : function( e ){
    e.preventDefault();
    m.route( uri, true );
  }
}

If that's too much boilerplate (ie you're using this pattern often), you might consider writing a little helper:

function replaceHistory( vdom ){
  vdom.onclick = function( e ){
    e.preventDefault();
    m.route( uri, true );
  };

  return vdom;
}

// And then
m( 'li', replaceHistory( m( 'a', { href : '/some/place' }, 'Follow my link' ) ) )
lhorie commented 8 years ago

Going to go ahead and close this since the workaround works and I think a replace mode for config: m.route is different enough from expected link behavior that it probably shouldn't be a thing