-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Double slash problem on card pop-ups #785
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
Comments
My initial attempt was to update the remote flow_router repo to my own repo named serubin:flow_router. This however required me to update every subsequent repo that used any version of flow_router. I also ran into some odd dependency issues and decided it wasn't worth the effort. My second attempt (which I've only just begun and am working on fixing up some meteor environment issues) is to add kadira:flow_router as a local package and let meteor use that over the remote versions. meteor hasn't been able to see my local package yet, but I'm still working to fix this. |
I'd be happy to pitch in anyway I can. I spent most of the day trying to come up with an Nginx rewrite that would fix it and failed so far. I'm unfamiliar with meteor and flow router, but I'm wondering if it's possible to get off of the call to absoluteUri() and use a relative path in the view instead. |
Currently the most realistic thing to do seems to be adding my fork of flowrouter to a "local package." The flow router fix requires two packages, one of the main branch and another of the ssr branch. flowrouter separates the two for someone reason and requires both. So far I've been unable to get meteor to see my local packages though. I've been dealing with some other stuff in life and haven't had time to investigate further into this issue. |
The current issue with flow-router is this: var completePath = this.path.apply(this, arguments);
// /wekan/b
var basePath = this._basePath || '/';
// /wekan
var pathWithoutBase = completePath.replace(new RegExp('^' + basePath), '');
// /b
return Meteor.absoluteUrl(pathWithoutBase);
// https://mydomain.net/wekan//b The router should definitely be taking out the leading slash. Changing line 7 of lib/router.js of flowrouter to var pathWithoutBase = completePath.replace(new RegExp('^(\/)|' + basePath), ''); An argument could be added for keeping that leading slash if necessary (ie. using a hashbang). |
@Serubin I've tried to get your fork working, but I'm running into issues with it. I've spent some time debugging and I think I know where it's failing. I haven't been able to figure out how to make it work, but I'm sure you'll be able to. |
@Marabon That sounds about right. I made a mistake in my branch. var pathWithoutBase = completePath.replace(new RegExp('^' + basePath + '(\/)'), ''); Feel free to fix, I'm without my laptop (due to repairs) for the moment so I wont be able to get that working just yet. |
Thanks, that new line works perfectly with my scenario of domain.tld/wekan! |
That's fantastic news! I've got to mention it though... it's not great to rely on a fork like this. I did try to get attention of the FlowRouter folks, but it does seem to be unmaintained at this point. Are there other options to replace it? I'm happy to open a new issue if y'all feel it's more appropriate. |
Excellent!
Much appreciated the effort! Solves my scenario as well, cheers!
El dia 18/03/2017 03:34, "Christopher McClellan" <notifications@github.com>
va escriure:
… That's fantastic news!
I've got to mention it though... it's not great to rely on a fork like
this. I did try to get attention of the FlowRouter folks, but it does seem
to be unmaintained at this point. Are there other options to replace it?
I'm happy to open a new issue if y'all feel it's more appropriate.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#785 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AD_HCcBW6kT7E7ZOJl7WltRm3CnZozSVks5rm0K9gaJpZM4L4VVg>
.
|
@Marabon Are you using my version of the router in wekan? @rubberduck203 The idea is to move it over to the wekan namespace once everything is working correctly again. |
@Serubin I couldn't get it to work simply adding your flow-router and removing the kadira one so I ended up cloning the kadira one to a local packages dir and simply applying your fix. |
I'll edit my repo shortly. I'm going to see if I can replicate what you did on my end and get that fix working and merged shortly |
Thank you so much Serubin!! |
@marcelofpfelix is there any chance you can share your rewrite rule? I spent a day trying to create one that worked. |
@rubberduck203 @marcelofpfelix Unfortunately because wekan is asynchronous the rewrite rule won't work. The urls are interpreted on the client and not the server so it will never get passed through apache or nginx to allow for a rewrite to work. |
You're right @Serubin. My workaround was to put the ROOT_URL in http. Since I have a Redirect rule from http to https in apache, the url pass through apache and the rewrite is loaded. But this is also not working for me, since the page reload takes time and and sidebar is set to hidden xD |
@Marabon Could you help me out and provide a step by step of how you added my fix to the project? I'm struggling to meteor to recognize my local package. |
@Serubin I created a dir in my home dir named localpackages where I cloned kadirahq:flow-router to. I then edited the file for your fix. (I'm guessing the same would work for your package, but as previously mentioned the project was looking for kadira so had to name the local package that) I had to set an environment variable in my .bashrc and .profile to make meteor aware of the local packages. (At first it didn't work when I tried to add the variable but that's because meteor looks for a different variable after a certain version, and we're using a version before that change)
I also made the version of the local package one version higher than the original one to make sure that it found and used the right one. Hope this helps. |
@Marabon Perfect. Thank you |
Moved here from wefork#49
Summary
The card's pop-up links to URL like:
http://192.168.30.240/wekan//b/Gtw5WWes4kvanupHu/devel/HduAsvhiCJLPJF4Hb
Note the double slash //b, if I remove one slash, it works as expected. Also, navigating to the boards doesn't suffer from this.
This issue affects Wekan running behind webserver (Apache/Nginx/Caddy) proxy. Wekan only works when using locally without proxy, for example like http://localhost:3000 .
@Serubin tried to get that FlowRouter fix kadirahq/flow-router#691 merged, with process like forking multiple other repos that would then be hosted under Wekan GitHub organization, but was not able to make Wekan build process working correctly.
For more details, see previous discussion at wefork#49
The text was updated successfully, but these errors were encountered: