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

Lift up debug page #4599

Merged
merged 6 commits into from
Feb 28, 2023
Merged

Lift up debug page #4599

merged 6 commits into from
Feb 28, 2023

Conversation

Dogacel
Copy link
Contributor

@Dogacel Dogacel commented Dec 31, 2022

Motivation:

Debug page is not very easy navigate, it is in the bottom most part of the application. I think debug page can have its own page, so I have implemented a pop-up that allows you to open debug page (more like a form) while browsing.

Modifications:

  • Remove debug page from bottom of the method page.
  • Add Debug button near the method name for easier access which opens a pop-up
  • Submit and Copy as cURL buttons stick to the bottom now.
  • Make information card about adding example requests dismissible.
Screen.Recording.2022-12-31.at.14.24.18.mov

Result:

Future improvements:

We are close to implementing JSONSchema support in Armeria. I think this will greatly improve the way developers write code inside the debug form, they won't even need to close the debug form to check the method body because IDE will provide comments + autocomplete features.

@Dogacel Dogacel changed the title lift up debug page Lift up debug page Dec 31, 2022
Copy link
Contributor

@ikhoon ikhoon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, looks good! Left minor comments.

@ikhoon ikhoon added this to the 1.22.0 milestone Jan 3, 2023
Copy link
Contributor

@minwoox minwoox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM once @ikhoon's comments are addressed. 😉

@Dogacel
Copy link
Contributor Author

Dogacel commented Jan 3, 2023

Just made a little more artistic touch here:

image

Now empty space is being used more efficiently.

Full changelog:

  • Make debug page lift up by clicking a button instead of putting it on the bottom of the page.
  • Make warning about specifying example requests dismissable.
  • Put a divider between request/response fields.
  • Trim some space on top of the Endpoints section.
  • Remove Section component so the whole thing expanded a little bit.
  • Make Header section one-liner as default to save some space.

@@ -161,7 +179,7 @@ const DebugPage: React.FunctionComponent<Props> = ({
if (useRequestBody) {
if (urlParams.has('request_body')) {
urlRequestBody = jsonPrettify(urlParams.get('request_body')!);
scrollToDebugForm();
setDebugFormIsOpen(true);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In addition to request_body, headers, endpoint_path and queries could be set to urlParams. How about showing the debug form if urlParams has values?

Copy link
Contributor

@jrhee17 jrhee17 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! Thanks @Dogacel ! 🙇 👍 🚀

Copy link
Contributor

@ikhoon ikhoon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @Dogacel! 🙇‍♂️

Would you mind resolving the conflicts?

@Dogacel
Copy link
Contributor Author

Dogacel commented Jan 31, 2023

Thanks, @Dogacel! 🙇‍♂️

Would you mind resolving the conflicts?

Sure, I was OOO for almost 2 weeks for travel. I will take a look when I am avail. I will re-test them too sometimes resolving conflicts are not enough by itself. I'll post my update tomorrow 😉

@Dogacel
Copy link
Contributor Author

Dogacel commented Jan 31, 2023

Thanks, @Dogacel! 🙇‍♂️

Would you mind resolving the conflicts?

Fixed a bug that caused debug form to open up whenever a method is clicked. Re-tested and merged latest master now it is working fine.

@ikhoon
Copy link
Contributor

ikhoon commented Feb 5, 2023

Fixed a bug that caused debug form to open up whenever a method is clicked.

I found out that it causes another regression that a debug form is closed immediately when submit button is clicked. I tested it by setting /service/multi to the endpoint path of AnnotatedDocServiceTest.MyService.multi() in doc service.

@ikhoon ikhoon modified the milestones: 1.22.0, 1.23.0 Feb 8, 2023
@ikhoon
Copy link
Contributor

ikhoon commented Feb 8, 2023

We are about to release a new version. Let me reschedule this to 1.23.0. 🙇‍♂️

@Dogacel
Copy link
Contributor Author

Dogacel commented Feb 8, 2023

multi

This should fix

    setDebugFormIsOpen(isOpen => isOpen || urlRequestBody !== '' || urlQueries !== '' ||);

If form is already opened, URL parameters shouldn't affect the state because of || debugFormIsOpen.

Sending the updated commit...

Edit: I have reproduced & fixed the issue with this last commit. Thanks for the heads up 😄

@ikhoon ikhoon self-requested a review February 9, 2023 03:58
@Dogacel Dogacel requested review from jrhee17 and removed request for trustin and ikhoon February 22, 2023 13:37
@Dogacel Dogacel requested review from minwoox and ikhoon and removed request for trustin, ikhoon, jrhee17 and minwoox February 22, 2023 13:37
Copy link
Contributor

@ikhoon ikhoon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beautiful work! 👨‍🎨 💯

@ikhoon ikhoon added this pull request to the merge queue Feb 28, 2023
Merged via the queue into line:master with commit ff25d6f Feb 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants