[SPARK-29570][WEBUI] Improve tooltip for Executor Tab for Shuffle Write,Blacklisted,Logs,Threaddump columns #26263

07ARB · 2019-10-25T21:40:27Z

What changes were proposed in this pull request?

All tooltips message will display in centre.

Why are the changes needed?

Some time tooltips will hide the data of column and tooltips display position will be inconsistent in UI.

Does this PR introduce any user-facing change?

yes.

How was this patch tested?

Manual test.

…te,Blacklisted,Logs,Threaddump columns ### What changes were proposed in this pull request? All tooltips message will display in center. ### Why are the changes needed? Some time tooltips will hide the data of column and tooltips display position will be inconsistent in UI. ### Does this PR introduce any user-facing change? yes. ### How was this patch tested? Manual test. Authored-by: Ankit Raj Boudh <ankitrajboudh@gmail.com>

07ARB · 2019-10-27T21:36:22Z

@srowen please help me to review this PR

srowen · 2019-10-27T22:49:28Z

How about we standardize on "auto" everywhere? is that the default? It's good to standardize but I think we can let the browser decide, as the right placement will depend on where the elements show in the window.

07ARB · 2019-10-28T12:47:15Z

I think that's also fine, I will change to auto.

Fixed review comments given by srowen,changed data-placement from top to auto.

07ARB · 2019-10-29T11:01:46Z

@srowen, I have fixed the review comments,please check.

srowen

Are these all the data-placement occurrences?

srowen · 2019-10-29T13:40:53Z

core/src/main/scala/org/apache/spark/ui/jobs/StageTable.scala

        val headerSpan = tooltip.map { case (title, left) =>
          if (left) {
-            /* Place the shuffle write tooltip on the left (rather than the default position
+            /* Place the shuffle write tooltip on the auto (rather than the default position


Nit: we need to reword this comment: "Place the tooltip automatically rather than on top because..."
Actually you don't need the if-else here anymore?
It might be worth double-checking that the auto placement on this element works well.

@07ARB I think we can make one more change here to simplify.

@srowen, auto is not working properly ,tooltips will display always in top-left corner of browser (chrome).

That's weird! OK then let's standardize on anything that displays reasonably everywhere. I think a few of these worked differently on purpose to avoid display issues though; maybe we leave any special logic as-is.

…te,Blacklisted,Logs,Threaddump columns ### What changes were proposed in this pull request? All tooltips message will display in center. ### Why are the changes needed? Some time tooltips will hide the data of column and tooltips display position will be inconsistent in UI. ### Does this PR introduce any user-facing change? yes. ### How was this patch tested? Manual test. Authored-by: Ankit Raj Boudh

…RK-29570

srowen

Generally I'm just wondering if it results in tooltip placement that is not worse in all cases. Standardization is good; in some cases a different placement might still be needed.

srowen · 2019-11-06T23:17:40Z

core/src/main/scala/org/apache/spark/ui/jobs/StageTable.scala

-            the tooltip is wider than the column, so it doesn't fit on top. */
-            <span data-toggle="tooltip" data-placement="left" title={title}>
+        val headerSpan = if (null != tooltip && !tooltip.isEmpty) {
+            <span data-toggle="tooltip" data-placement="top" title={tooltip}>


This is an example of a case where the placement appears to be on purpose. How does this look before and after?

ok i will give you the screenshot

Before :

After my Change :

@srowen , please check this if you still feel it's not require then we can close this PR.

AbhishekNew · 2019-11-08T08:33:05Z

Display tooltip at center is standard. Looks good to me.

SparkQA · 2019-11-09T16:33:46Z

Test build #4917 has finished for PR 26263 at commit 1a75e4d.

This patch fails Spark unit tests.
This patch does not merge cleanly.
This patch adds no public classes.

SparkQA · 2019-11-10T20:50:27Z

Test build #4918 has finished for PR 26263 at commit 1a75e4d.

This patch fails Spark unit tests.
This patch does not merge cleanly.
This patch adds no public classes.

07ARB · 2019-11-11T04:37:04Z

Failure test cases not related to this PR.

SparkQA · 2019-11-11T17:52:15Z

Test build #4922 has finished for PR 26263 at commit 1a75e4d.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2019-11-11T18:38:49Z

Test build #4923 has finished for PR 26263 at commit 1a75e4d.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2019-11-11T20:33:33Z

Test build #4927 has finished for PR 26263 at commit 1a75e4d.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2019-11-11T20:54:50Z

Test build #4926 has finished for PR 26263 at commit 1a75e4d.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

srowen · 2019-11-13T00:50:11Z

Merged to master

07ARB · 2019-11-13T02:29:35Z

@srowen , Thank you for your guidance and i really feel good working with you.

dongjoon-hyun added the WEB UI label Oct 26, 2019

07ARB added 10 commits October 29, 2019 02:51

Changed data-placement from top to auto.

43320f5

Fixed review comments given by srowen,changed data-placement from top to auto.

Changed data-placement from top to auto.

578686b

Fixed review comments given by srowen,changed data-placement from top to auto.

Changed data-placement from top to auto.

6e427ce

Fixed review comments given by srowen,changed data-placement from top to auto.

Changed data-placement from top to auto.

32a0ff1

Fixed review comments given by srowen,changed data-placement from top to auto.

Changed data-placement from top to auto.

c4389af

Fixed review comments given by srowen,changed data-placement from top to auto.

Changed data-placement from top to auto.

04af940

Fixed review comments given by srowen,changed data-placement from top to auto.

Changed data-placement from top to auto.

15f06e0

Fixed review comments given by srowen,changed data-placement from top to auto.

Changed data-placement from top to auto.

21a9295

Fixed review comments given by srowen,changed data-placement from top to auto.

changed data-placement from top to auto.

033f578

Fixed review comments given by srowen,changed data-placement from top to auto.

changed data-placement from top to auto.

9ae8d75

Fixed review comments given by srowen,changed data-placement from top to auto.

srowen reviewed Oct 29, 2019

View reviewed changes

07ARB added 4 commits November 7, 2019 03:27

Merge branch 'SPARK-29570' of https://github.com/07ARB/spark into SPA…

fb484b2

…RK-29570

review comments fixed

f10e8e7

review comments fixed

590eab1

07ARB requested a review from srowen November 6, 2019 23:15

srowen reviewed Nov 6, 2019

View reviewed changes

07ARB added 4 commits November 7, 2019 21:44

fixed review comments.

cf7dca9

fixed review comments.

45d23e3

fixed review comments.

63fa48b

review comments fixed.

1a75e4d

07ARB requested a review from srowen November 8, 2019 04:47

07ARB mentioned this pull request Nov 9, 2019

[SPARK-29453][WEBUI] Improve tooltips information for SQL tab. #26259

Closed

srowen approved these changes Nov 12, 2019

View reviewed changes

srowen closed this in 45e212e Nov 13, 2019

[SPARK-29570][WEBUI] Improve tooltip for Executor Tab for Shuffle Write,Blacklisted,Logs,Threaddump columns #26263

[SPARK-29570][WEBUI] Improve tooltip for Executor Tab for Shuffle Write,Blacklisted,Logs,Threaddump columns #26263

Uh oh!

Conversation

07ARB commented Oct 25, 2019

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

07ARB commented Oct 27, 2019

Uh oh!

srowen commented Oct 27, 2019

Uh oh!

07ARB commented Oct 28, 2019

Uh oh!

07ARB commented Oct 29, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

srowen left a comment

Choose a reason for hiding this comment

Uh oh!

srowen Oct 29, 2019

Choose a reason for hiding this comment

Uh oh!

srowen Nov 2, 2019

Choose a reason for hiding this comment

Uh oh!

07ARB Nov 6, 2019

Choose a reason for hiding this comment

Uh oh!

srowen Nov 6, 2019

Choose a reason for hiding this comment

Uh oh!

srowen left a comment

Choose a reason for hiding this comment

Uh oh!

srowen Nov 6, 2019

Choose a reason for hiding this comment

Uh oh!

07ARB Nov 6, 2019

Choose a reason for hiding this comment

Uh oh!

07ARB Nov 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

07ARB Nov 8, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AbhishekNew commented Nov 8, 2019

Uh oh!

SparkQA commented Nov 9, 2019

Uh oh!

SparkQA commented Nov 10, 2019

Uh oh!

07ARB commented Nov 11, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SparkQA commented Nov 11, 2019

Uh oh!

SparkQA commented Nov 11, 2019

Uh oh!

SparkQA commented Nov 11, 2019

Uh oh!

SparkQA commented Nov 11, 2019

Uh oh!

srowen commented Nov 13, 2019

Uh oh!

07ARB commented Nov 13, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

07ARB commented Oct 29, 2019 •

edited

Loading

07ARB Nov 6, 2019 •

edited

Loading

07ARB Nov 8, 2019 •

edited

Loading

07ARB commented Nov 11, 2019 •

edited

Loading