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

routeUrl helper, merge option not working #2291

Closed
gmahomarf opened this Issue Feb 25, 2016 · 8 comments

Comments

Projects
None yet
2 participants
@gmahomarf

gmahomarf commented Feb 25, 2016

I'm trying to use {{routeUrl(page='home', true)}} to change only the page attribute in my route, but it's not working. Debugging shows that when we call the helper @here, params is true and merge is the options object, as defined in the stache helpers docs. Shouldn't those arguments be inverted, so that they match the documentation (i.e. all arguments first, then options object with hash)?

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Feb 25, 2016

Contributor

When you use a call expression, there really shouldn't be an options object.

Instead page='home' should pass an object like {page: "home"} as the first argument and true as the second.

Contributor

justinbmeyer commented Feb 25, 2016

When you use a call expression, there really shouldn't be an options object.

Instead page='home' should pass an object like {page: "home"} as the first argument and true as the second.

@gmahomarf

This comment has been minimized.

Show comment
Hide comment
@gmahomarf

gmahomarf Feb 25, 2016

I'll try it, thanks. I was using a call expression as detailed in the routeUrl helper docs. They should be updated.

gmahomarf commented Feb 25, 2016

I'll try it, thanks. I was using a call expression as detailed in the routeUrl helper docs. They should be updated.

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Feb 25, 2016

Contributor

@gmahomarf I'm a bit confused. Does it not work the way I suggested in my comment?

If yes, then this is a bug, not a problem with the docs.

Contributor

justinbmeyer commented Feb 25, 2016

@gmahomarf I'm a bit confused. Does it not work the way I suggested in my comment?

If yes, then this is a bug, not a problem with the docs.

@gmahomarf

This comment has been minimized.

Show comment
Hide comment
@gmahomarf

gmahomarf Feb 25, 2016

@justinbmeyer just to be clear, you're suggesting I do the following?:

<a href="{{routeUrl({page: 'home'}, true)}}"> Link</a>

gmahomarf commented Feb 25, 2016

@justinbmeyer just to be clear, you're suggesting I do the following?:

<a href="{{routeUrl({page: 'home'}, true)}}"> Link</a>
@gmahomarf

This comment has been minimized.

Show comment
Hide comment
@gmahomarf

gmahomarf Feb 25, 2016

@justinbmeyer I ran several tests this morning. Each example has the stache template where routeUrl was used. Below that are values of the arguments sent to the routeUrl helper (see code) and the resulting value for the href attribute.

To avoid cluttering the issue comments, I created a gist with the results: https://gist.github.com/gmahomarf/92d97265694f0c1a77ec

I hope this helps

gmahomarf commented Feb 25, 2016

@justinbmeyer I ran several tests this morning. Each example has the stache template where routeUrl was used. Below that are values of the arguments sent to the routeUrl helper (see code) and the resulting value for the href attribute.

To avoid cluttering the issue comments, I created a gist with the results: https://gist.github.com/gmahomarf/92d97265694f0c1a77ec

I hope this helps

@gmahomarf

This comment has been minimized.

Show comment
Hide comment
@gmahomarf

gmahomarf Feb 29, 2016

Any news on this?

gmahomarf commented Feb 29, 2016

Any news on this?

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Mar 1, 2016

Contributor

Canjs is not a priority for me this week. I'm working on can-set issues currently. If you want to help fix it, I can pair with you this week and close it out. Otherwise it will probably have to wait until next week.

Sent from my iPhone

On Feb 29, 2016, at 5:46 PM, Gazy Mahomar notifications@github.com wrote:

Any news on this?


Reply to this email directly or view it on GitHub.

Contributor

justinbmeyer commented Mar 1, 2016

Canjs is not a priority for me this week. I'm working on can-set issues currently. If you want to help fix it, I can pair with you this week and close it out. Otherwise it will probably have to wait until next week.

Sent from my iPhone

On Feb 29, 2016, at 5:46 PM, Gazy Mahomar notifications@github.com wrote:

Any news on this?


Reply to this email directly or view it on GitHub.

@gmahomarf

This comment has been minimized.

Show comment
Hide comment
@gmahomarf

gmahomarf Mar 1, 2016

@justinbmeyer For now, I've got my code working as I need it, so this is no longer urgent for me. Ping me whenever you have time to take a look at this, and I'd love to help fix it. Thanks!

gmahomarf commented Mar 1, 2016

@justinbmeyer For now, I've got my code working as I need it, so this is no longer urgent for me. Ping me whenever you have time to take a look at this, and I'd love to help fix it. Thanks!

@justinbmeyer justinbmeyer self-assigned this Mar 4, 2016

@justinbmeyer justinbmeyer added the bug label Mar 4, 2016

@justinbmeyer justinbmeyer added this to the 2.3.19 milestone Mar 4, 2016

justinbmeyer added a commit that referenced this issue Mar 4, 2016

@daffl daffl closed this in #2307 Mar 4, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment