-
Notifications
You must be signed in to change notification settings - Fork 1
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
ENG: Add HTML Response View #57
base: main
Are you sure you want to change the base?
Conversation
Thanks for rendering this in an iframe! That was going to be my one request. |
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.
This is pretty awesome.
Thanks for adding the backend server that we can use for examples as well. I'll test this weekend and bump the tag 🙌
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.
This looks great, will test soon!
Co-authored-by: Matthew P. Kerle <[email protected]>
Socket Security ReportDependency issues detected. If you merge this pull request, you will not be alerted to the instances of these issues again. 📜 New install scripts detectedA dependency change in this PR is introducing new install scripts to your install step.
🫣 Native codeContains native code which could be a vector to obscure malicious code, and generally decrease the likelihood of reproducible or reliable installs.
Socket.dev scan summary
Bot CommandsTo ignore an alert, reply with a comment starting with
Powered by socket.dev |
|
||
return ( | ||
<div> | ||
<ApiResponseStatus status={response.status} /> |
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.
@jwdotjs Was just fixing a merge conflict on this and wanted to be sure you intended to replace what was here with this ApiResponseStatus
. It was missing from your changes here: https://github.com/smartrent/badmagic/pull/76/files#diff-1f1f2427fe79e8a1ee3be3aa06af1e81516c424970d69b021dfad9342eb26174L39
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.
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.
spooky 👻 where is it coming from 😂
I'll remove it
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.
Other than the duplicated response status this works for successful requests that return HTML. Do we want to also allow rendering for error responses?
Screen.Recording.2022-10-07.at.2.31.09.PM.mov
// schema: { | ||
// type: "array", | ||
// items: { | ||
// $ref: "#/definitions/Dog", | ||
// }, | ||
// }, |
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.
?
|
||
return ( | ||
<div> | ||
<ApiResponseStatus status={response.status} /> |
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.
Description
Found reading the raw html particularly frustrating when creating new routes and you have error responses from Phoenix which are unreadable. I was copying the text to a file to view in the browser but this is much faster 😄
This adds an option to view a rendered html response for a route. It also adds a basic local test server for development.
The response is not rendered by default and it is rendered in an iframe for sandboxing.
Steps to test
example
folder./views/breeds.html
route.Screenshots