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

Restore "Request URL" in live response section #3421

Merged
merged 7 commits into from Sep 29, 2017
Merged

Restore "Request URL" in live response section #3421

merged 7 commits into from Sep 29, 2017

Conversation

dalbani
Copy link
Contributor

@dalbani dalbani commented Jul 20, 2017

I found the "request URL" section quite useful in Swagger UI v2, for example to replicate the request in a another browser tab or any other tool. (Evidently, that only works for GET requests.)
This is my proposal to restore this functionality in Swagger UI v3.
In terms of implementation, I wasn't sure if an <a> or <pre> was more appropriate — or even a <textarea> with auto copy-paste as for the Curl section.

@webron
Copy link
Contributor

webron commented Jul 20, 2017

Wouldn't that be equivalent to just copying the request URL from the curl sample?

@dalbani
Copy link
Contributor Author

dalbani commented Jul 21, 2017

@webron: yes, sure, this additional section is only there to make easier to copy the URL.
In practice, that's the difference between finding the URL within curl -X GET "http://petstore.swagger.io/v2/pet/22" -H "accept: application/xml" or simply copy from a section containing http://petstore.swagger.io/v2/pet/22 only.

@shockey
Copy link
Contributor

shockey commented Aug 22, 2017

I made some changes:

  • Removed RequestUrl component...
    • "Request" URL seemed like a misnomer, since the URL is actually gleaned from the response. But I'm ok with keeping that for backwards compatibility expectations... @webron feel free to override this. The only edge case I can think of where the URL would differ between request and response, is following a Location header.
    • A new component seemed like overkill for an h4 + textarea, when we have an even bigger section inlined in the commponent below. Not saying breaking it off was a bad idea.. just that it's good to be consistent with what gets its own component.
  • The URL display now triggers based on url being available, instead of curlRequest.

@shockey
Copy link
Contributor

shockey commented Aug 22, 2017

I'm approving the code here, leaving the feature up to @webron. The request vs response naming here should not be overlooked before merging.

@webron
Copy link
Contributor

webron commented Sep 11, 2017

@shockey request url is correct in this case. I'm ok with merging it, even though not a huge fan.

@shockey shockey merged commit c6a6643 into swagger-api:master Sep 29, 2017
@dalbani dalbani deleted the feature/request-url branch November 22, 2017 12:46
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.

None yet

3 participants