Handle Ajax form submission with Turbolinks (#133) #1133
Conversation
|
Will this possibly also fix #1093? |
It looks like it fix, in my test app this problem is missing |
There was a problem hiding this comment.
This is awesome! It'll be nice to have this a little more solid. I'm not too familiar with turbolinks though, so I'm not sure what all is involved here. Looking at the linked issues to turbolinks and rails, this all looks good. I just left a few comments on things I'm wondering about.
| context.response.headers.add "Turbolinks-Location", path | ||
| context.response.status_code = status | ||
| Lucky::TextResponse.new(context, "", "") | ||
| if request.headers["X-Requested-With"]?.try(&.downcase) == "xmlhttprequest" && request.method != "GET" |
There was a problem hiding this comment.
We also do this same line here https://github.com/luckyframework/lucky/blob/master/src/lucky/mime_type.cr#L106. Maybe this can be used somehow? Or play back in to this thing https://github.com/luckyframework/lucky/blob/master/src/lucky/request_type_helpers.cr
There was a problem hiding this comment.
first method is private
about second, it not works for me #1131
There was a problem hiding this comment.
@skojin After this would you mind opening another PR that uses this logic for the ajax? check? If not that's ok, just wondering :)
| RedirectAction.new(context, params).call | ||
| context.response.status_code.should eq 200 | ||
| context.response.headers["Turbolinks-Location"].should eq "/somewhere" | ||
| context.cookies.deleted?(:_turbolinks_location).should be_true |
* Handle Turbolinks-Location header in correct way
| context.response.headers.add "Turbolinks-Location", path | ||
| context.response.status_code = status | ||
| Lucky::TextResponse.new(context, "", "") | ||
| if request.headers["X-Requested-With"]?.try(&.downcase) == "xmlhttprequest" && request.method != "GET" |
There was a problem hiding this comment.
@skojin After this would you mind opening another PR that uses this logic for the ajax? check? If not that's ok, just wondering :)
Implements #133
Tested on demo app.
Checklist
crystal tool format spec src./script/setup./script/test