veliovgroup / flow-router

🚦 Carefully extended flow-router for Meteor
https://packosphere.com/ostrio/flow-router-extra
BSD 3-Clause "New" or "Revised" License
202 stars 29 forks source link

allow to disable automatic link detection #52

Closed macrozone closed 6 years ago

macrozone commented 6 years ago

I moving that over from the original flow router repo:

https://github.com/kadirahq/flow-router/issues/705

I try to update to this version here, but my monkey patch from the original flow router no longer works, because _page is now hidden behind a getter :-(

So we need to adjust that in the initialisation code.

Maybe i can do a PR for that

dr-dimitru commented 6 years ago

Hello @macrozone ,

Do you wish to make _page static on FlowRouter object?

dr-dimitru commented 6 years ago

@macrozone or we can even add an option to disable binding for click event. Do you have thoughts for PR?

macrozone commented 6 years ago

@dr-dimitru, yes, I would do the following:

will try that out know

EDIT: its probably better to pass it as option to initialize:

initialize({disableLinkDetection: true})

dr-dimitru commented 6 years ago

@macrozone I believe it should be initialize option

dr-dimitru commented 6 years ago

Like:

FlowRouter.wait();
WhenEverYourAppIsReady(() => {
  FlowRouter.initialize({
    page: {
      click: false
    }
  });
});
dr-dimitru commented 6 years ago

@macrozone we can even allow this way to pass other options to page package

macrozone commented 6 years ago

@dr-dimitru there aren't that much options in page.js and one is already in options: hashbang. I don't think there are much more options that we ever want to pass to page.js ;-)

page.js is an implementation detail anyway.

But i will try out if it works for me and then we can decide how the option should look like, both variants are ok for me

dr-dimitru commented 6 years ago

@macrozone thank you a lot for contribution. Going to test on my end and publish later tonight

dr-dimitru commented 6 years ago

@macrozone just realized there was a way to access ._page:

import { Router } from 'meteor/ostrio:flow-router-extra';
Router.prototype._page; // <---

Added full list of exported classes and instances.

macrozone commented 6 years ago

@dr-dimitru good to know, but i am glad that we have now a clean solution for that specific problem that does not rely on monkey-patching ;-)

dr-dimitru commented 6 years ago

@macrozone thank you a lot for contribution. Sorry for delay with publishing, had to finish accumulated tasks 😬

Published as v3.5.1 Feel free to reopen it in case if the issue is still persists on your end.