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

Fix namespacing issues for links in render helpers #53

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

shekibobo
Copy link

  • d3c0e0d removed the ability to use namespaces on the sortable tree helper. This reinstates that capability, but also adds consistent rendering and namespacing for all render helpers.
  • ce696f3 meant to clarify that a show link was not being used for a delete link, but neglected to update in all the helpers. This PR also addresses that issue.
  • Actually, this PR makes each renderer consistent in how it builds links, and allows you to explicitly pass what controller you want to use for building said links.

@the-teacher
Copy link
Owner

looks fine - I will try it on my dummy app - if it's works correct - I will be glad to merge it

thank you!

@shekibobo
Copy link
Author

Just so you know, I may have one or two more adjustments coming through on separate PRs today. I'm trying to get this working with ActiveAdmin, and there are a few issues I'm working through...

@shekibobo
Copy link
Author

Actually, I lied. I'm including that as part of this PR. Instead of trying to infer the link controller from the klass option, we first check to see if the user passed a controller option, and we use that instead.

Turns out all that does is add an extra url parameter
so that `namespace` after the `?` in the URL. Not what I
was going for. Apparently, simply building the url for
the given controller seems sufficient for building the proper link.
@shekibobo
Copy link
Author

So I'm not sure exactly how this is working, but namespace is never actually used. I have namespaced routes (under /admin using ActiveAdmin), and after removing the namespace option from my tree renderer call, it still works fine. However, my controller is named (without a namespace) to make it work properly, since my class name does not match my controller name.

@shekibobo
Copy link
Author

It turns out that adding the namespace options just added the value to the URL query string, which was not what the end goal was.

@the-teacher
Copy link
Owner

It was good PR. I have to keep it in mind if I will improve the gem.
Thanks again!

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.

2 participants