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

Feat: Tool tip support for larger values in Table visualization #6009

Closed
wants to merge 2 commits into from

Conversation

Nctdt
Copy link
Contributor

@Nctdt Nctdt commented Mar 20, 2023

Pull Request Description

Fixes #5968
When the content exceeds the width and causes additional content to be displayed as "...", hovering the mouse for one second will use a tooltip to display all the content.

1679314293639
1679314316214
1679314326304

Important Notes

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • The documentation has been updated, if necessary.
  • Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
  • All code follows the
    Scala,
    Java,
    and
    Rust
    style guides. In case you are using a language not listed above, follow the Rust style guide.
  • All code has been tested:
    • Unit tests have been written where possible.
    • If GUI codebase was changed, the GUI was tested when built using ./run ide build.

@Nctdt Nctdt added the CI: No changelog needed Do not require a changelog entry for this PR. label Mar 20, 2023
Copy link
Member

@wdanilo wdanilo left a comment

Choose a reason for hiding this comment

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

How does it look with really long text, e.g. one that doesn't fit in one line? Also, how does it look like with text that has many lines, eg. 1000?

@Nctdt
Copy link
Contributor Author

Nctdt commented Mar 21, 2023

How does it look with really long text, e.g. one that doesn't fit in one line? Also, how does it look like with text that has many lines, eg. 1000?

Displaying all the content can make it very long, but this can be resolved by reducing the displayed content, such as reducing it to a single line or 100 characters or less. How do you think it should be handled?

@wdanilo
Copy link
Member

wdanilo commented Mar 22, 2023

How does it look with really long text, e.g. one that doesn't fit in one line? Also, how does it look like with text that has many lines, eg. 1000?

Displaying all the content can make it very long, but this can be resolved by reducing the displayed content, such as reducing it to a single line or 100 characters or less. How do you think it should be handled?

We should show the user as much info as possible. So the popup should wrap the data and should display as much as possible there in multiple lines. Basically, the more data user is able to preview, the better. Also, @sylwiabr, @NedHarding, and @jdunkerley, what if the multiline preview is not enough? Should the preview be scrollable?

@sylwiabr
Copy link
Member

I don't think we need a scrollable preview in a tooltip.

@Nctdt
Copy link
Contributor Author

Nctdt commented Mar 23, 2023

My idea is to truncate the string to 50 characters and display ${str.slice(0, 50)}... if it exceeds the limit. What do you think, @wdanilo ?

@sylwiabr sylwiabr requested a review from jdunkerley March 27, 2023 11:41
@wdanilo
Copy link
Member

wdanilo commented Mar 27, 2023

If we could at least show long string broken into several lines (like 4-5 lines) that would be amazing. Then, sure, let's just skip rest of the string and add ... at the end to show the user that there is more. Ok? :)

@NedHarding
Copy link

This gets so tricky and subtle. What if the string ends in "..."? Its always a good plan to use some other color/symbol to make meta information like the string being truncated separate than the data itself. This is true both in the grid and in the tooltip version of the display, which presumably truncate at different lengths.

Also, it is really important for a user to be able to explore the entirety of the data. The simplest way this can be accomplished is to allow copy to the clipboard to get the full text. But then you are asking the user to switch to external software to analyze their data - so they (the user) might question why they are using enso and not the other program they had to switch to?

And we have to remember, really long values are really common. It is not an exceptional case at all.

@wdanilo
Copy link
Member

wdanilo commented Mar 28, 2023

I totally agree with @NedHarding, that's why I proposed scrolling the values in popups. Copy-pasting them to another tools looks like a really bad solution, while scroll would solve all issues here.

@NedHarding
Copy link

I mostly agree with @wdanilo - except the tooltip part. Scrollbars and selection for copy/paste are much easier with a persistent control instead of a tooltip. Tooltips have this difficult habit of going away if you move the mouse wrong. To effectively browse a large field and scroll and potentially copy a selection, it would be better served by a text box that is accessed by a double click on a value and then it shows below/right or on top of grid and has to be actively dismissed. This also makes a keyboard interface more intuitive (enter or space) where tooltips are typically mouse only.

@wdanilo
Copy link
Member

wdanilo commented Mar 28, 2023

Oh, thats exactly what I meant by tooltip. You are right, my terminology was wrong. Thanks for catching it and explaining it so clearly!

@NedHarding
Copy link

@wdanilo I should add - there still needs to be some maximum to avoid crashing the GUI. It is very easy to construct a value with >4GB for instance which we would not want to load in memory. So even with scrolling, we should cap at 1MB or something sane for a text control.

},
tooltipShowDelay: 1000,
tooltipMouseTrack: true,
enableRangeSelection: true,
Copy link
Member

Choose a reason for hiding this comment

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

This will result in warnings in the console if not running against "Enterprise" AG Grid.

Copy link
Contributor

Choose a reason for hiding this comment

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

@jdunkerley we use enterprise AGGrid now, don't we? So perhaps we could resurrect this PR and move changes to Table Vis in GUI2?

@PabloBuchu
Copy link
Contributor

@Nctdt whats the status of this PR? When you can address all the issues?

@Nctdt
Copy link
Contributor Author

Nctdt commented Apr 11, 2023

@Nctdt whats the status of this PR? When you can address all the issues?

I'm sorry, I cannot solve these problems.

@farmaazon
Copy link
Contributor

@PabloBuchu @Nctdt What are the action points?

If the problems are unsolvable, can we close this PR?

@jdunkerley
Copy link
Member

Lets close for now - we have a different approach suggested by @NedHarding which we will implement in the future.

@farmaazon farmaazon closed this May 30, 2023
@somebody1234 somebody1234 deleted the wip/lsy/large-value-tooltip branch December 7, 2023 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: No changelog needed Do not require a changelog entry for this PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tool tip support for larger values in Table visualization
7 participants