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

Polish Edit & Resend Panel #121

Open
janodvarko opened this issue Apr 16, 2020 · 24 comments
Open

Polish Edit & Resend Panel #121

janodvarko opened this issue Apr 16, 2020 · 24 comments

Comments

@janodvarko
Copy link
Member

janodvarko commented Apr 16, 2020

Context

The Edit & Resend panel is used to modify an existing HTTP Request and resend it back to the server. Here is a screenshot of the current UI:

image

The current UI allows the user to modify:

  • HTTP method
  • URL
  • Query string/arguments
  • Request headers
  • Request body (aka POST data)

Existing issues are collected under this meta: Bug 1475161 - [meta] Edit and Resend

One of the things we'd like to fix first is related to URL encoding/decoding issues. Especially, the UI isn't user friendly when it comes to editing (encoded) query arguments.

Bug 987975 - edit and resend doesn't properly encode query string values

There is mockup for better UI here

Prior Art

There are also some Chrome extensions introducing UI for editing query strings.

The attached screenshots are using this example URL:

http://example.com/index.html?someval=5oMEnMSQqJ8%3D&another==value%3D

Easy URL Params

https://chrome.google.com/webstore/detail/easy-url-params/jhhjlajfdkofbnhbcjcddngfehfihfln

image

Easy URL Editor

https://chrome.google.com/webstore/detail/easy-url-editor/kojpdadnbbicdfgfadonheclfpcjpiah?hl=en

image

@digitarald @violasong @bomsy

Honza

@digitarald
Copy link
Contributor

digitarald commented Apr 16, 2020

If people here generally agree on the approach in the wireframes (that basically just map out key/value form input pairs); the scope of this UX issue would be mostly about the UI polish to correctly style the buttons/inputs.

image

cc @belen-albeza, do you see any issues with this approach?

@digitarald
Copy link
Contributor

cc @fvsch as well who had some excellent input on forms in the past.

@violasong
Copy link
Contributor

Is the idea behind the checkboxes that we would now save your inputs between sessions, and allow you to disable the ones you're not currently using?

And sounds like we'll be moving the form to the left sidebar? Is this because it's helpful to be able to see the right sidebar at the same time?

It would be nice to have a bit more of a visual connection between the param and value fields, like what's in the prior art screens. It would be nice to show the columns on one line always, though if we're putting this in the left sidebar, that might be more likely to get too limited in space. But something like this could be nice:

[________] : [_________________]

@digitarald
Copy link
Contributor

Is the idea behind the checkboxes that we would now save your inputs between sessions, and allow you to disable the ones you're not currently using?

Yes, a way to disable/enable entries to explore and iterate quickly.

And sounds like we'll be moving the form to the left sidebar? Is this because it's helpful to be able to see the right sidebar at the same time?

Just to clarify, this would be follow-up. Left sidebar allows to keep the form open while repeatedly firing requests. Right now sending a new request closes the form to focus the response.

[] : [_________]

+1, I actually had that in my wireframes as wider option. I just didn't think about making the first input smaller, which is a good idea.

@janodvarko
Copy link
Member Author

Just to clarify, this would be follow-up. Left sidebar allows to keep the form open while repeatedly firing requests. Right now sending a new request closes the form to focus the response.

Note that this is part of our 20% project and @juliandescottes is ready to work on this soon:
Bug 1618069 - Move Network's "New Request" page into left sidebar

It might be quite independent work from the polish and could be done in parallel. The move itself is relatively straightforward. The more complex part is the Form relation to the selected (and fake) request.

The current logic is creating a temporary request (when the Edit & Resend form is opened) that is used to store data from the form. We should break that connection (form <-> selected request) since it's buggy and the implementation is hacky.

Try e.g.

  1. Open the form on a request
  2. select the original request again
  3. Open the form again
  4. Now there are multiple fake requests in the list with the same IDs so, both are selected at the same time (and there are React warnings about dup keys)
  5. Cancel the form, now there is forgotten temp request.
  6. Continue doing this and the list turns into a mess.

We should:

  1. Break the connection with the current selection. We might have another UI indication to point to the original request used to resent (as suggested by Julian)
  2. Store state of the form properly in a reducer. So, we can save the inputs between sessions (as suggested by Harald)
  3. Stop creating temporary requests with fake IDs and remove all hacks around this from the code base (this would be great cleanup)

I think that Julian can start analysis on this as part of the 20% project and perhaps even provide a patch if there is enough time.

Honza

@digitarald
Copy link
Contributor

Break the connection with the current selection. We might have another UI indication to point to the original request used to resent (as suggested by Julian)

To simplify scope, we might skip that. This way "Edit & Resend" just becomes "Pre-populate the create request form with this request's details". If we move the form to the sidebar, the original request would still be selected until the user submits, I assume? But maybe I am missing why we should show the original request.

@violasong
Copy link
Contributor

+1, I actually had that in my wireframes as wider option. I just didn't think about making the first input smaller, which is a good idea.

Yep, that looked good - I think we could also make the association between key/value fields a bit stronger, perhaps like one of these:

image

Also, seems like (with the current min-width on this sidebar) we have enough room to always do it side-by-side.

@janodvarko
Copy link
Member Author

To simplify scope, we might skip that. This way "Edit & Resend" just becomes "Pre-populate the create request form with this request's details". If we move the form to the sidebar, the original request would still be selected until the user submits, I assume?

This concept would work for me.

I am also attaching a screenshot from Postman for some more inspiration (https://www.postman.com/). A tool used to Test REST API (so, basically send custom requests)

image

Some notes:

  • See the method field, it's a drop down allowing the user to pick different HTTP method (POST, GET, etc.)
  • Note the Hid auto-generated headers switch. Those headers are hidden by default. Not sure if we have any auto-generated headers though.
  • See the little (i) icon it shows a tooltip that explains purpose of the header and why it should be set.
  • Btw. list of headers UI looks exactly the same as list of query string. List of query strings is edited in the Params tab
  • When an invalid value for header name is inserted there is red cross button.
  • The hamburger menu in front of the Content-Length allows changing the order by drag-and-drop
  • The request response body is displayed at the bottom of the form as soon as the request is sent (and the body received). This is really handy and we can achieve the same UX by auto-selecting the sent request and if the Response panel (on the right side is visible) it would show the response right away.
  • Note the Bulk Edit button that switched the mode into raw so, the user can just type headers in multi-line text area (allows nice copy-paste)

@digitarald
Copy link
Contributor

See the method field, it's a drop down allowing the user to pick different HTTP method (POST, GET, etc.)

Agreed, I thought I am missing the reason why method should be a free form input as a dropdown makes a lot more sense.

We should provide autocompletion for headers as well, as we have a well-defined list and datalist on the input can provide a quick way to autocomplete.

Note the Hid auto-generated headers switch. Those headers are hidden by default. Not sure if we have any auto-generated headers though.

Which ones would that be, @janodvarko? I would have assumed Content-Length, but its in the list – so I guess its values from other panels like Authorization.

See the little (i) icon it shows a tooltip that explains purpose of the header and why it should be set.

MDN integration for picked headers would be great, as devs can look up the right format and other details.

Note the Bulk Edit button that switched the mode into raw so, the user can just type headers in multi-line text area (allows nice copy-paste)

Copy/pasting is an important use case, so devs are not forced into the split input format. This could also be another place for a Raw toggle.

@janodvarko
Copy link
Member Author

We should provide autocompletion for headers as well, as we have a well-defined list and datalist on the input can provide a quick way to autocomplete.

Yes, I like the idea. @nchevobbe can we somehow share the auto-completion logic that you are building for classes in the Rules panel? We could ideally just provide different datalist ....

Note the Hide auto-generated headers switch. Those headers are hidden by default. Not sure if we have any auto-generated headers though.

Which ones would that be, @janodvarko? I would have assumed Content-Length, but its in the list – so I guess its values from other panels like Authorization.

All the headers with gray background (on the screenshot) are auto-added. Postman allows to define presets - groups of headers that should be auto-added. Not sure whether we want this.

But, authorization is one of the examples where headers are auto-added.

See the next screenshot of the Authorization tab and the comment under API Key
image

Note the Bulk Edit button that switched the mode into raw so, the user can just type headers in multi-line text area (allows nice copy-paste)

Copy/pasting is an important use case, so devs are not forced into the split input format. This could also be another place for a Raw toggle.

And we have SourceEditor that @bomsy implemented that can be used with + HTTP headers syntax color highlighting.

Some related comments are here https://bugzilla.mozilla.org/show_bug.cgi?id=1632030#c2

Honza

@nchevobbe
Copy link
Member

Yes, I like the idea. @nchevobbe can we somehow share the auto-completion logic that you are building for classes in the Rules panel? We could ideally just provide different datalist ....

We're building on top of the shared autocomplete popup, on which Razvan suggested we add the most common keyboard shortcut so it's easier to use from any given input.
I guess in this case we won't even have to call the server, as the list does not depend on the content page, but on a set of known items right?

@janodvarko
Copy link
Member Author

I guess in this case we won't even have to call the server, as the list does not depend on the content page, but on a set of known items right?

Correct, the list of headers will be hardcoded.
We already have list of headers supported by MDN here:
https://searchfox.org/mozilla-central/rev/55a4faa52f72918efa51150d127aebdc057dc6cf/devtools/client/netmonitor/src/utils/mdn-utils.js#15

Honza

@violasong
Copy link
Contributor

@janodvarko
Copy link
Member Author

Thanks for the mockups Victoria, it looks great!

image

Some comments:

  1. The (top) input field for URL is short and I think that it'll be hard to edit (even slightly) longer URLs and query string arguments.

We have nice URL expandable preview component (implemented for the Headers panel) So, what if we make it editable and reuse that in this case? The user could either edit the raw URL string or expand and edit its parts separately: method, protocol, domain, file path, query string arguments.

  1. What about reducing the amount of lines/borders/rectangles by using an editable table?
    Or perhaps could we reuse the design we have:
  • for listing Watch Expressions in the Debugger panel (with checkboxes like we have for list of BPs).
  • for listing request/response headers in the Headers panel
    (the input boxes are created when selecting an entry)

But, I understand that the responsive layout might be important UX part since side bars tends to be rather thinner than wider.

Honza

@violasong
Copy link
Contributor

Collecting feedback on the latest mockups

@violasong
Copy link
Contributor

Made some small changes based on the latest feedback. Also grabbed more feedback from twitter as a safety check.

  • Added "payload" placeholder text
  • Made the Send button a little larger
  • Autocomplete suggestion for known header names would be great
  • Moving the Send button at top was suggested, but I think bottom is a bit better for the user flow.
  • Someone asked if we really need plus buttons if new fields appear automatically. Probably not, assuming we also have god support for pasting in a bunch of headers. I've hidden them for now.

I think this mockup is feeling pretty solid :) - let me know if there's anything else I should do!

@janodvarko
Copy link
Member Author

janodvarko commented May 25, 2020

I really like how it looks like on the last mockups, thanks @violasong !

assuming we also have god support for pasting in a bunch of headers.

I'd do it inline with what we have for the Blocking panel

  1. The user focuses the empty name:value row (the one for creating a new header)
  2. The user pastes list of new-line separated list of headers (name:value pairs)

e.g.

Accept-Ranges: bytes
Cache-Control: max-age=30
Connection: Keep-Alive

Ideally the same thing (if not we should file a bug), which is copied from the Headers panel

  1. List of headers is created in the UI.

The same for URL arguments (including the copy from the existing Headers panel part)

Honza

@violasong
Copy link
Contributor

Sounds great! Nice to see the blocking behavior already recognizes line breaks so nicely.

@violasong
Copy link
Contributor

Mockup with new revisions:

  • Removed "URL/Method" header
  • Added Clear button
  • Proposing + as its toolbar button

@janodvarko
Copy link
Member Author

Looks great, thanks Victoria.

Proposing + as its toolbar button

I don't see anything related on the mockups. Where should I look?

Honza

@violasong
Copy link
Contributor

violasong commented Jun 1, 2020

I don't see anything related on the mockups. Where should I look?

I haven't made a full mockup of this but here's what the + icon would look like:
image

@brunetton
Copy link

Any news on this ? I miss this feature (a lot !)

@janodvarko
Copy link
Member Author

janodvarko commented Dec 17, 2021

Work on this is currently in progress. Should be ready at the end of Feb 2022
You might follow this meta bug
https://bugzilla.mozilla.org/show_bug.cgi?id=1475161

@brunetton
Copy link

Work on this is currently in progress. Should be ready at the end of Feb 2022 You might follow this meta bug https://bugzilla.mozilla.org/show_bug.cgi?id=1475161

Thanks ! In fact I realized yesterday that my Firefox version wasn't up to date; and I tried with 93.0 and it's already there ! 🥳

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

5 participants