Skip to content

Improve design of profiler and add support for formatting request/response body #245

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

Closed
ostrolucky opened this issue Jan 6, 2018 · 9 comments

Comments

@ostrolucky
Copy link
Collaborator

ostrolucky commented Jan 6, 2018

Q A
Bug? no
New Feature? yes

Please compare how does profiler look like between these bundles. csa/guzzle-bundle is A LOT clearer.

csa/guzzle-bundlephp-http/httplug-bundle
csa/guzzle-bundle profiler php-http/httplug-bundle profiler

I suggest:

  1. Syntax highlight json responses (even better, xml too)
  2. Don't put request/response side-by-side
  3. Use table for headers

edit: Syntax highlighting/formatting might look like a hard part. It isn't, look at referenced bundle. Detection and formatting, syntax highlight is done by prismjs

@dbu
Copy link
Collaborator

dbu commented Jan 7, 2018

+1 for headers as table. syntax highlighting could indeed be cheap with some js library. the side-by-side could also be a responsive layout to switch below a certain screen size.

do you want to start with a pull request for headers in a table? that sounds the most quickwin to me.

for syntax highlighting, i am +1 even though it means bundling some js library to do that.

@fbourigault
Copy link
Contributor

Request and response are displayed side by side because when you toggle the plugin execution, you can read starting from the left going down each plugin change in the request and then go up on the right side to see how plugins modify the response. Also, syntax highlighting would be great but be careful as the content may be truncated depending on the body length captured while profiling.

@dbu
Copy link
Collaborator

dbu commented Jan 7, 2018

Request and response are displayed side by side because when you toggle the plugin execution, you can read starting from the left ...

ah right. wdyt about having the csa bundle layout first, but once you activate the plugin display, it switches to our current side-by-side layout? maybe additionally responsive so that on really large screens, we always get side by side?

@fbourigault
Copy link
Contributor

Yes, we could have a different layout for the main display.

@ostrolucky
Copy link
Collaborator Author

ostrolucky commented Jan 7, 2018

I'm not frontend developer. If you don't want to get rid of vertically split layout, classical tables might not be best fit. For instance, in posted screenshot one of the header keys is very long.

2szwodw8je

Using classical tables would mean first column will be as wide as largest element is, which would leave very little space for second column. Of course, you can prevent that by setting max-width for first column, but now you are mixing there static variables, that's not good.

It would be better if this is built properly by someone who is better in frontend than me. I guess using flexbox? I don't know. Maybe tables would be really best way, want to make sure though.

@dbu
Copy link
Collaborator

dbu commented Jan 8, 2018

i guess we would have to do a maxwidth on both columns of the table and shorten too long headers (in css). if you have a designer who wants to work on it, please do...

@dbu
Copy link
Collaborator

dbu commented Dec 27, 2019

fixed in #355

@dbu dbu closed this as completed Dec 27, 2019
@ostrolucky
Copy link
Collaborator Author

Not really, only partially. Syntax highlighting of JSON body is missing.

@dbu
Copy link
Collaborator

dbu commented Dec 27, 2019

oh, right. i consider that an optional thing - if you want to contribute it, please do, but i don't think we need an open issue until you (or someone else) wants to pick it up and add the feature.

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

No branches or pull requests

3 participants