Scroll down to the debug form when a DocService URL contains … #2045

SooJungDev · 2019-09-05T01:05:49Z

Motivation:

User has to manually scroll down the method page to see the debug form.

Modifications:

Add move scroll function

Result:

without scrolling down to it.
closed Scroll down to the debug form when a DocService URL contains a request. #1682

CLAassistant · 2019-09-05T01:06:00Z

All committers have signed the CLA.

ikhoon · 2019-09-05T01:08:54Z

@SooJungDev Thanks for contributing Armeria! 👍

trustin · 2019-09-05T02:48:52Z

docs-client/src/containers/MethodPage/DebugPage.tsx

@@ -68,6 +68,8 @@ interface State {
 type Props = OwnProps & RouteComponentProps;

 class DebugPage extends React.PureComponent<Props, State> {
+  private scrollRef = React.createRef <HTMLDivElement>();


Question: Is it possible to define a meaningful anchor name to the reference at DOM level, such as debug-form?

Yes , I'll renaming variable. Thank you for review!

Oh, does just renaming the variable change the anchor ID in DOM?

trustin · 2019-09-05T02:49:24Z

docs-client/src/containers/MethodPage/DebugPage.tsx

@@ -589,6 +595,13 @@ class DebugPage extends React.PureComponent<Props, State> {
      debugResponse,
    });
  }
+
+  private moveScroll() {


How about renaming to scrollToDebugForm() to make it more meaningful?

I'll renaming method! Thank you for review!

docs-client/src/containers/MethodPage/DebugPage.tsx

ikhoon · 2019-09-06T00:07:27Z

@SooJungDev To pass CI build, you might want to check tslint. https://travis-ci.com/line/armeria/jobs/231309728#L397

ikhoon · 2019-09-07T01:30:34Z

@SooJungDev While I am seeing your commit log and git diff, I thought your branch is mixed with git merge and git rebase --merge, so that this diff shows dependency upgrade work which was done by @minwoox

Could you squash and revise your commits? 😀

ikhoon · 2019-09-07T06:52:15Z

It seems to have failed to rebase the master branch exactly... 😢
We should not see the Implement ProtocolDetectionHandler.exceptionCaught() commit.

ikhoon · 2019-09-08T05:25:40Z

@SooJungDev Finally, could you clean up duplicated commit message? :-)

... (+1 squashed commit)
Squashed commits:
[821c3ea] Motivation:
- User has to manually scroll down the method page to see the debug form.

Modifications:
- Add move scroll function

Result:
-  without scrolling down to it.
-  closed #1682

SooJungDev · 2019-09-08T05:48:53Z

@SooJungDev Finally, could you clean up duplicated commit message? :-)

... (+1 squashed commit)
Squashed commits:
[821c3ea] Motivation:
- User has to manually scroll down the method page to see the debug form.

Modifications:
- Add move scroll function

Result:
-  without scrolling down to it.
-  closed #1682

@SooJungDev Finally, could you clean up duplicated commit message? :-)

... (+1 squashed commit)
Squashed commits:
[821c3ea] Motivation:
- User has to manually scroll down the method page to see the debug form.

Modifications:
- Add move scroll function

Result:
-  without scrolling down to it.
-  closed #1682

ikhoon

Thanks @SooJungDev 👍

ikhoon · 2019-09-08T10:34:04Z

Please check tslint to pass CI build. 😿 https://travis-ci.com/line/armeria/builds/126346564#L399

…quest (#2045) Motivation: - User has to manually scroll down the method page to see the debug form. Modifications: - Add move scroll function Result: - without scrolling down to it. - closed #1682

codecov · 2019-09-08T12:05:51Z

Codecov Report

Merging #2045 into master will increase coverage by 0.02%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master    #2045      +/-   ##
============================================
+ Coverage     73.77%   73.79%   +0.02%     
- Complexity     9388     9389       +1     
============================================
  Files           817      817              
  Lines         36121    36121              
  Branches       4453     4453              
============================================
+ Hits          26648    26656       +8     
+ Misses         7168     7159       -9     
- Partials       2305     2306       +1

Impacted Files	Coverage Δ	Complexity Δ
.../linecorp/armeria/internal/IdleTimeoutHandler.java	`80% <0%> (-20%)`	`4% <0%> (-1%)`
...inecorp/armeria/server/HttpResponseSubscriber.java	`82.46% <0%> (-1.9%)`	`61% <0%> (-4%)`
.../main/java/com/linecorp/armeria/server/Server.java	`81.58% <0%> (-1.26%)`	`26% <0%> (ø)`
.../linecorp/armeria/client/Http2ResponseDecoder.java	`61.66% <0%> (+0.83%)`	`33% <0%> (+1%)`	⬆️
...inecorp/armeria/server/grpc/ArmeriaServerCall.java	`88.18% <0%> (+1.57%)`	`82% <0%> (+1%)`	⬆️
...corp/armeria/common/stream/FixedStreamMessage.java	`89.28% <0%> (+1.78%)`	`23% <0%> (+1%)`	⬆️
...necorp/armeria/server/GracefulShutdownSupport.java	`100% <0%> (+2.56%)`	`4% <0%> (ø)`	⬇️
.../linecorp/armeria/internal/Http2GoAwayHandler.java	`56.41% <0%> (+2.56%)`	`14% <0%> (+1%)`	⬆️
...om/linecorp/armeria/server/jetty/JettyService.java	`66% <0%> (+2.95%)`	`24% <0%> (+1%)`	⬆️
...meria/common/stream/RegularFixedStreamMessage.java	`87.75% <0%> (+6.12%)`	`14% <0%> (+1%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ad03af2...21d4db0. Read the comment docs.

trustin

Good job!

…2045) a request. Motivation: - User has to manually scroll down the method page to see the debug form when the URL contains a request. Modifications: - Scroll down to the debug form if the URL contains a request. Result: - No need to scroll down manually - Closes line#1682

SooJungDev requested review from ikhoon, minwoox and trustin as code owners September 5, 2019 01:05

ikhoon added the improvement label Sep 5, 2019

ikhoon added this to the 0.91.0 milestone Sep 5, 2019

trustin requested changes Sep 5, 2019

View reviewed changes

ikhoon requested changes Sep 5, 2019

View reviewed changes

docs-client/src/containers/MethodPage/DebugPage.tsx Outdated Show resolved Hide resolved

docs-client/src/containers/MethodPage/DebugPage.tsx Outdated Show resolved Hide resolved

minwoox modified the milestones: 0.91.0, 0.92.0 Sep 6, 2019

SooJungDev closed this Sep 8, 2019

SooJungDev reopened this Sep 8, 2019

ikhoon approved these changes Sep 8, 2019

View reviewed changes

Add scroll down to the debug form when a DocService URL contains a re…

21d4db0

…quest (#2045) Motivation: - User has to manually scroll down the method page to see the debug form. Modifications: - Add move scroll function Result: - without scrolling down to it. - closed #1682

trustin approved these changes Sep 10, 2019

View reviewed changes

trustin changed the title ~~Add scroll down to the debug form when a DocService URL contains a re…~~ Scroll down to the debug form when a DocService URL contains … Sep 10, 2019

trustin merged commit 84abd79 into line:master Sep 10, 2019

SooJungDev deleted the feature-addMoveScrollForDocService branch September 10, 2019 03:28

ikhoon mentioned this pull request Oct 7, 2019

Fix a bug where a method page scrolls down to its debug form when the service list is collapsed #2158

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Scroll down to the debug form when a DocService URL contains … #2045

Scroll down to the debug form when a DocService URL contains … #2045

SooJungDev commented Sep 5, 2019 •

edited

Loading

CLAassistant commented Sep 5, 2019 •

edited

Loading

ikhoon commented Sep 5, 2019

trustin Sep 5, 2019

SooJungDev Sep 5, 2019

trustin Sep 5, 2019

trustin Sep 5, 2019

SooJungDev Sep 5, 2019

ikhoon commented Sep 6, 2019

ikhoon commented Sep 7, 2019 •

edited

Loading

ikhoon commented Sep 7, 2019 •

edited

Loading

ikhoon commented Sep 8, 2019 •

edited

Loading

SooJungDev commented Sep 8, 2019

ikhoon left a comment

ikhoon commented Sep 8, 2019 •

edited

Loading

codecov bot commented Sep 8, 2019

trustin left a comment

Scroll down to the debug form when a DocService URL contains … #2045

Scroll down to the debug form when a DocService URL contains … #2045

Conversation

SooJungDev commented Sep 5, 2019 • edited Loading

CLAassistant commented Sep 5, 2019 • edited Loading

ikhoon commented Sep 5, 2019

trustin Sep 5, 2019

Choose a reason for hiding this comment

SooJungDev Sep 5, 2019

Choose a reason for hiding this comment

trustin Sep 5, 2019

Choose a reason for hiding this comment

trustin Sep 5, 2019

Choose a reason for hiding this comment

SooJungDev Sep 5, 2019

Choose a reason for hiding this comment

ikhoon commented Sep 6, 2019

ikhoon commented Sep 7, 2019 • edited Loading

ikhoon commented Sep 7, 2019 • edited Loading

ikhoon commented Sep 8, 2019 • edited Loading

SooJungDev commented Sep 8, 2019

ikhoon left a comment

Choose a reason for hiding this comment

ikhoon commented Sep 8, 2019 • edited Loading

codecov bot commented Sep 8, 2019

Codecov Report

trustin left a comment

Choose a reason for hiding this comment

SooJungDev commented Sep 5, 2019 •

edited

Loading

CLAassistant commented Sep 5, 2019 •

edited

Loading

ikhoon commented Sep 7, 2019 •

edited

Loading

ikhoon commented Sep 7, 2019 •

edited

Loading

ikhoon commented Sep 8, 2019 •

edited

Loading

ikhoon commented Sep 8, 2019 •

edited

Loading