-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Feature/3389/search resource dialogs #3393
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
Conversation
|
|
||
| @resource_area_selected = from_dialog? | ||
|
|
||
| render @callback.presence || 'insert' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @parndt ! I don't find how i could render the template by context insert or in /refinery/admin/pages_dialogs/link_to template.
Using @callback is not the good way, could you help me to find?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops you accidentally mentioned the @callback user 😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops! i will not expect a callback from this user 😆
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any idea @parndt ?
| @web_address_text = "http://" | ||
| @web_address_text = params[:current_link] if params[:current_link].to_s =~ /^http:\/\// | ||
| @web_address_text = "https://" | ||
| @web_address_text = params[:current_link] if params[:current_link].to_s =~ /^https:\/\// |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't the s be optional? Like /^https?:\/\//
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right
|
reopened to activate code climate |
020b254 to
10a6eee
Compare
|
Hi @parndt, could you review my PR? Also, i think the PR config of codeclimate is too strict, could we just warning us? |
|
The code looks fine. I think the code climate failures provide good points 😉 |
|
I dont't really know if we still need these lines : refinerycms/resources/app/controllers/refinery/admin/resources_controller.rb Lines 49 to 55 in 10a6eee
I don't really know if it still works since There is no spec for this lines : https://github.com/refinery/refinerycms/search?q=condition&unscoped_q=condition |
|
So @parndt will you acceptthis PR as it? :) |
|
Yeah. I think that the other things can be done as separate refactors. I was looking at it the other night, but it was quite intense. |
This fixes #3389