Skip to content

Conversation

@scwf
Copy link
Contributor

@scwf scwf commented Dec 29, 2014

In some case the job description will be very long, such as a long sql. refer to #3718
This PR add a pop-up for job description when it is long.

image

image

@SparkQA
Copy link

SparkQA commented Dec 29, 2014

Test build #24853 has started for PR 3819 at commit 4909bb4.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Dec 29, 2014

Test build #24853 has finished for PR 3819 at commit 4909bb4.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24853/
Test FAILed.

@SparkQA
Copy link

SparkQA commented Dec 29, 2014

Test build #24854 has started for PR 3819 at commit 8fe74b0.

  • This patch merges cleanly.

@scwf scwf changed the title [SPARK-4984][Core][UI] Adding a pop-up containing the full for job description when it is very long [SPARK-4984][CORE][UI] Adding a pop-up containing the full for job description when it is very long Dec 29, 2014
@scwf scwf changed the title [SPARK-4984][CORE][UI] Adding a pop-up containing the full for job description when it is very long [SPARK-4984][CORE][UI] Adding a pop-up containing the full job description when it is very long Dec 29, 2014
@SparkQA
Copy link

SparkQA commented Dec 29, 2014

Test build #24854 has finished for PR 3819 at commit 8fe74b0.

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

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24854/
Test PASSed.

@srowen
Copy link
Member

srowen commented Dec 29, 2014

This can be turned into a simple mouseover with much less work, no CSS or Javascript. Just display the shortened version, and make the long description the title attribute of an enclosing tag like span or div. I think that might even be more intuitive?

@scwf
Copy link
Contributor Author

scwf commented Dec 29, 2014

Hi @srowen, I am not a expert of css/javascript, my question here if do like your suggest(mouseover), can we copy the full sql statement in ui? sometime we want to copy it.

@srowen
Copy link
Member

srowen commented Dec 29, 2014

A mouseover can have whatever you like, whatever you can put in a title attribute. The browser will lay it out. You could try and see if it's an effective view.

@scwf
Copy link
Contributor Author

scwf commented Dec 29, 2014

thanks, i will try with it:)

@scwf
Copy link
Contributor Author

scwf commented Dec 29, 2014

hi @srowen, i tried like this

<div title="full sql: ....">shortened sql</div>

but i can not copy the "full sql", or i missed something?

@srowen
Copy link
Member

srowen commented Dec 29, 2014

@scwf You should be able to put whatever you want in there as the title. What's the issue?

@scwf
Copy link
Contributor Author

scwf commented Dec 29, 2014

Sorry, did not get you. Do you mean i put others here( to implement an attribute similar with title) instead of title?

@srowen
Copy link
Member

srowen commented Dec 29, 2014

I'm suggesting something like replacing

<div><em>{lastStageDescription}</em></div>

with

<div title="{lastStageDescription}"><em>{shortLastStageDescription}</em></div>

... when the description is long. This should cause the short description to pop up the full description in a mouseover. Maybe I'm missing something as to why that won't work, but it is a lot simpler at least.

@scwf
Copy link
Contributor Author

scwf commented Dec 29, 2014

Hi @srowen, this and <div title="full sql: ....">shortened sql</div> both work for me. Why i do not use this solution is that we maybe want to copy(ctrl + c) the full description from UI sometimes, if using title we can not copy the full description

But this is really much simpler and if you think it's very low probability for user to copy the full desc, i will change it to this simple solution:)

@scwf
Copy link
Contributor Author

scwf commented Jan 12, 2015

/cc @JoshRosen, can you have a look at this?

@scwf
Copy link
Contributor Author

scwf commented Jan 14, 2015

Ping @JoshRosen, any comment here?

@JoshRosen
Copy link
Contributor

What do you think about using a styled read-only text form field? Using CSS, we could probably limit the size of what's displayed while still allowing users to double-click to copy the entire field.

@scwf scwf force-pushed the popup-descrip-ui branch from 8fe74b0 to 4b55c3b Compare January 16, 2015 09:08
@SparkQA
Copy link

SparkQA commented Jan 16, 2015

Test build #25649 has started for PR 3819 at commit 0bca96d.

  • This patch merges cleanly.

@scwf
Copy link
Contributor Author

scwf commented Jan 16, 2015

@JoshRosen Using a read-only text form, the screenshot is like this, also support to copy the description.

image

@SparkQA
Copy link

SparkQA commented Jan 16, 2015

Test build #25649 has finished for PR 3819 at commit 0bca96d.

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

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25649/
Test PASSed.

@scwf
Copy link
Contributor Author

scwf commented Jan 19, 2015

Hmm, agree with you, but have not found a easy way to spot truncated description.
if we add ... for truncated decs, we will consider the case of window scaling and different screen which make this more complex

@scwf
Copy link
Contributor Author

scwf commented Jan 20, 2015

Updated @JoshRosen, now use ... to spot it's been truncated
image

image

@SparkQA
Copy link

SparkQA commented Jan 20, 2015

Test build #25818 has started for PR 3819 at commit a7c5e7b.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Jan 20, 2015

Test build #25818 has finished for PR 3819 at commit a7c5e7b.

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

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25818/
Test PASSed.

@JoshRosen
Copy link
Contributor

Ooh, very nice! I didn't know about text-overflow: ellipsis. Looks like it has good browser support, too: http://caniuse.com/#feat=text-overflow

@JoshRosen
Copy link
Contributor

I just tried this out in Firefox and it turns out that it won't let you select text from a disabled input element. Apparently this has been a longstanding issue; this bug has been open for 11+ years: https://bugzilla.mozilla.org/show_bug.cgi?id=195361

@JoshRosen
Copy link
Contributor

I think I found a solution that works in Firefox: replace the input element with a span where you've set it to display: block and white-space: nowrap. If we do that, I think we can drop the border, margin, padding, and cursor, and box shadow attributes.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why do these all need "!important"? Are there other rules that will take precedence here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, in my debug there is other style will take precedence here. anyway, new version i do not use input and remove "!important"

@SparkQA
Copy link

SparkQA commented Jan 21, 2015

Test build #25868 has started for PR 3819 at commit c5c4822.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Jan 21, 2015

Test build #25868 has finished for PR 3819 at commit c5c4822.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25868/
Test FAILed.

@scwf scwf force-pushed the popup-descrip-ui branch from c5c4822 to ba02b83 Compare January 21, 2015 03:11
@SparkQA
Copy link

SparkQA commented Jan 21, 2015

Test build #25869 has started for PR 3819 at commit ba02b83.

  • This patch merges cleanly.

@scwf
Copy link
Contributor Author

scwf commented Jan 21, 2015

@JoshRosen, yeah, get it, updated.

@SparkQA
Copy link

SparkQA commented Jan 21, 2015

Test build #25869 has finished for PR 3819 at commit ba02b83.

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

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25869/
Test PASSed.

@scwf
Copy link
Contributor Author

scwf commented Jan 21, 2015

Ping @JoshRosen is this ok?

@JoshRosen
Copy link
Contributor

Yep, looks good to me. Mind updating the screenshot in the PR description? Do that, ping me, and I'll merge it right away. Thanks!

@scwf
Copy link
Contributor Author

scwf commented Jan 21, 2015

Ping @JoshRosen updated done

@JoshRosen
Copy link
Contributor

Awesome, thanks! I'm merging this into master (1.3.0).

@asfgit asfgit closed this in 3be2a88 Jan 21, 2015
@scwf scwf deleted the popup-descrip-ui branch January 21, 2015 23:29
scwf added a commit to Huawei-Spark/spark that referenced this pull request Jan 22, 2015
…cription when it is very long

In some case the job description will be very long, such as a long sql. refer to apache#3718
This PR add a pop-up for job description when it is long.

![image](https://cloud.githubusercontent.com/assets/7018048/5847400/c757cbbc-a207-11e4-891f-528821c2e68d.png)

![image](https://cloud.githubusercontent.com/assets/7018048/5847409/d434b2b4-a207-11e4-8813-03a74b43d766.png)

Author: wangfei <[email protected]>

Closes apache#3819 from scwf/popup-descrip-ui and squashes the following commits:

ba02b83 [wangfei] address comments
a7c5e7b [wangfei] spot that it's been truncated
fbf6162 [wangfei] Merge branch 'master' into popup-descrip-ui
0bca96d [wangfei] remove no use val
4b55c3b [wangfei] fix style issue
353c6f4 [wangfei] pop up the description of job with a styled read-only text form field
@JoshRosen
Copy link
Contributor

I think that this feature may no longer be working in recent versions of Spark. Have you tested this recently?

@scwf
Copy link
Contributor Author

scwf commented Jul 7, 2015

i tested it locally today, seems it is ok.

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

Successfully merging this pull request may close these issues.

6 participants