Show visualization on output port hover. #1363

MichaelMauderer · 2021-03-23T15:01:51Z

Pull Request Description

Introduces a quick preview for visualizations and errors. Hovering a node output will first show a tooltip with the type information and then after some time, will show the visualization of the node. The preview visualization will be located above other nodes, whereas the normal view, will be shown below nodes.
Some notes: errors will show the preview visualization immediately. Nodes without type information will show the visualization immediately. If you press and hold ctrl tooltips are skipped and the preview visualization shows up immediately.

Peek.2021-03-23.16-27.mp4

Checklist

Please include the following checklist in your PR:

The CHANGELOG.md was updated with the changes introduced in this PR.
The documentation has been updated if necessary.
All code conforms to the Rust style guide.
~~All code has automatic tests where possible.~~
All code has been profiled where possible.
All code has been manually tested in the IDE.
~~All code has been manually tested in the "debug/interface" scene.~~
All code has been manually tested by the PR owner against our test scenarios.
All code has been manually tested by at least one reviewer against our test scenarios.

wdanilo

Michael, it looks amazing. I would definitely change two behaviors:

Nodes without type information will show the visualization immediately - this should not be here. The thing is that we do not show vis preview immediately in order not to have blinking in GUI, so no type information should not affect our visuals.
The error should be shown not only on output port hover, but just on this node hover.

CHANGELOG.md

wdanilo · 2021-03-24T10:11:32Z

src/rust/ensogl/lib/core/src/animation/frp/animation/delayed.rs

+            frp.source.on_reset <+ transition.value.map(|t| (t - 0.0).abs() < std::f32::EPSILON).on_true();
+            frp.source.on_end   <+ transition.value.map(|t| (t - 1.0).abs() < std::f32::EPSILON).on_true();


too long - please check all new lines in the codebase and check if they cross the 100chars boundary

src/rust/ide/view/graph-editor/src/component/node.rs

src/rust/ide/view/graph-editor/src/lib.rs

…put-port-hover

wdanilo · 2021-03-24T20:33:12Z

Michael, could you please also move the popup below the mouse, like it is on main now? I was playing with main today and it looks so beautiful and consistent with the popup below the mouse, not on the right? If you could just make it configurable (I think it is), it would be cool, so we will play with both versions

…put-port-hover

MichaelMauderer · 2021-03-25T12:11:17Z

Michael, could you please also move the popup below the mouse, like it is on main now? I was playing with main today and it looks so beautiful and consistent with the popup below the mouse, not on the right? If you could just make it configurable (I think it is), it would be cool, so we will play with both versions

Done. This can be configured on a per-tooltip basis. (See last commit on this branch)

mwu-tow

I've run the test scenarios. Here are the issues I've written down:

BASIC SCENARIO

When I press tab and start typing a new node (with no prior selection), the tooltip appear, despite not having content.
Tooltip should not appear while we are creating node.
Also, tooltip should never appear empty. It should either not appear in such cases or include some diagnostics, as to why it is empty.
Histogram visualization has broken panning. When I move mouse with MMB pressed, it quickly goes wild.
"Fit all" button in histogram stops working after too much panning is done.
Likewise, visualization zooming is broken when dragging with RMB pressed.
Engine reports execution failures: Execution failed in context f8ff239e-dc10-4521-b96c-ec507a128412. Error: Methods cannot be overloaded, but you have tried to overload Histogram.enso_project..
It is impossible to enter the node with func1 after adding a new defaulted parameter. (test scenario 1): error in console Error while performing UI action on controllers: The suggestion with id 1567 has not been found in the database.
It is impossible to connect additional input to func1 as there is no placeholder for it on node.
Due to above issues some testings steps have been omitted. Once issues are addressed, the full test scenario should be once more checked.

TRAMS SCENARIO

The table visualization does not appear on fields.get "vehicles". Can be triggerred though..
There is no such method as Geo.points.
The geo map visualization not only does not open bubt it is impossible to be selected (missing from visualziations list).
Further steps of the scenario are too sketchy for me to follow. Will need to chat with @sylwiabr later.

Some well known issues are omitted for clarity,

farmaazon · 2021-03-26T08:44:53Z

I've run the test scenarios. Here are the issues I've written down:

BASIC SCENARIO

Histogram visualization has broken panning. When I move mouse with MMB pressed, it quickly goes wild.

"Fit all" button in histogram stops working after too much panning is done.

I spotted those two also on my branch, so those are likely regressions on develop.

It is impossible to enter the node with func1 after adding a new defaulted parameter. (test scenario 1): error in console Error while performing UI action on controllers: The suggestion with id 1567 has not been found in the database.

It is impossible to connect additional input to func1 as there is no placeholder for it on node.

Those are engine issues and were reported some time ago.

TRAMS SCENARIO

The geo map visualization not only does not open bubt it is impossible to be selected (missing from visualziations list).

This also was reported as a develop regression

…put-port-hover

MichaelMauderer · 2021-03-29T07:38:14Z

1. When I press tab and start typing a new node (with no prior selection), the tooltip appear, despite not having content.
   Tooltip should not appear while we are creating node.
   Also, tooltip should never appear empty. It should either not appear in such cases or include some diagnostics, as to why it is empty.

This is somewhat fixed. The really proper solution would be to check whether the node has an output value from the engine, but that information is not currently available to the node. So now this only skips nodes that do not yet have content to avoid the pop-up on empty nodes.

2. Histogram visualization has broken panning. When I move mouse with MMB pressed, it quickly goes wild.

Regression unrelated to this PR.

3. "Fit all" button in histogram stops working after too much panning is done.

Regression unrelated to this PR.

4. Likewise, visualization zooming is broken when dragging with RMB pressed.

Regression unrelated to this PR.

5. Engine reports execution failures: `Execution failed in context f8ff239e-dc10-4521-b96c-ec507a128412. Error: Methods cannot be overloaded, but you have tried to overload Histogram.enso_project.`.

As Adam mentioned on stand-up: unrelated to this PR and under investigation.

MichaelMauderer · 2021-03-29T09:01:15Z

TRAMS SCENARIO

1. The table visualization does not appear on `fields.get "vehicles"`. Can be triggerred though..

Regression on develop. When opening the table vis on this node it opens up as empty for me.

2. There is no such method as `Geo.points`.

I believe there is only a Geo.point method. But I don't see either used in the Scenario. Can you point out where this happens?

farmaazon · 2021-03-29T09:18:51Z

@MichaelMauderer

I believe there is only a Geo.point method. But I don't see either used in the Scenario. Can you point out where this happens?

Yes, it was fixed when me and Michał went through the scenario together recently.

mwu-tow

@MichaelMauderer
I did the following:

start a new unnamed project
move nodes
save
reopen IDE
hover nodes

The first nodes has tooltip with type. The second node (sort) has no tooltip. Though it likely is caused by a factor external to this PR, as node output port is also missing colors. Still might be worth investigating, as this PR makes the issue more visible.

Observed this when I was about to perform the copying step of the scenario.

Visualization automatically appears when I try to add a node after File.read.

Enso.2021-03-30.11-06-55.mp4

I haven't tested further, once those issues are resolved (or confirmed as known issues unrelated to the PR) I will retest.
Please also merge develop, it should bring at least some fixes.

…put-port-hover

MichaelMauderer · 2021-03-30T10:09:07Z

@mwu-tow

@MichaelMauderer
I did the following:
* start a new unnamed project

* move nodes

* save

* reopen IDE

* hover nodes
The first nodes has tooltip with type. The second node (sort) has no tooltip. Though it likely is caused by a factor external to this PR, as node output port is also missing colors. Still might be worth investigating, as this PR makes the issue more visible.

This is expected behaviour, if the node has no type information (which would explain why it has no colour either). If there is no type information to display, no tooltip is shown. But otherwise this is a bug that should be reported as "Node that should have a type does not have a type".

Observed this when I was about to perform the copying step of the scenario.

I believe this has been addressed on Discord?

Visualization automatically appears when I try to add a node after File.read.
Enso.2021-03-30.11-06-55.mp4

It looks like you are still hovering another node, so this is expected behaviour unless we want to prevent hovering while editing a node.

I haven't tested further, once those issues are resolved (or confirmed as known issues unrelated to the PR) I will retest.
Please also merge develop, it should bring at least some fixes.

Branch is updated to newest develop.

mwu-tow · 2021-03-30T11:12:19Z

@MichaelMauderer

It looks like you are still hovering another node, so this is expected behaviour unless we want to prevent hovering while editing a node.

To me it looks like bug, even if it was designed this way. User must hover node to click on it to select. Then they open searcher to create a new node. And after a moment a strange popup appears, obscuring everything. They should not be required to move mouse away from node before opening searcher. This looks to me like a very common scenario.

Also, observe behavior on the video closely. At the moment when visualization appears, the output port is not hovered. It was hovered for a brief moment before, however mouse was moved and the type tooltip was hidden.

MichaelMauderer · 2021-03-30T13:11:05Z

It looks like you are still hovering another node, so this is expected behaviour unless we want to prevent hovering while editing a node.

To me it looks like bug, even if it was designed this way. User must hover node to click on it to select. Then they open searcher to create a new node. And after a moment a strange popup appears, obscuring everything. They should not be required to move mouse away from node before opening searcher. This looks to me like a very common scenario.

Also, observe behavior on the video closely. At the moment when visualization appears, the output port is not hovered. It was hovered for a brief moment before, however mouse was moved and the type tooltip was hidden.

@mwu-tow You are right! It seems the FRP endpoint I hooked into was not only triggered when hovering the output port but some additional area. This is fixed now.

…put-port-hover # Conflicts: # CHANGELOG.md # src/rust/ide/view/graph-editor/src/component/visualization/container.rs # src/rust/ide/view/graph-editor/src/lib.rs

mwu-tow · 2021-03-30T14:41:19Z

The issue seems resolved to me.

…put-port-hover

[ci build]

…put-port-hover

Original commit: enso-org/ide@b869971

MichaelMauderer added Difficulty: Core Contributor Should only be attempted by a core contributor Priority: High Should be scheduled as soon as possible Type: Enhancement An enhancement to the current state of Enso IDE Category: GUI The Graphical User Interface labels Mar 23, 2021

MichaelMauderer self-assigned this Mar 23, 2021

feat: Show visualization on output port hover.

d012500

MichaelMauderer force-pushed the wip/mm/ide-1302-show-visualization-on-output-port-hover branch from df53137 to f6ae178 Compare March 23, 2021 15:07

doc: Add changelog entry.

22d4e1c

MichaelMauderer force-pushed the wip/mm/ide-1302-show-visualization-on-output-port-hover branch from f6ae178 to 22d4e1c Compare March 23, 2021 15:31

MichaelMauderer marked this pull request as ready for review March 23, 2021 15:50

MichaelMauderer requested a review from wdanilo as a code owner March 23, 2021 15:50

wdanilo approved these changes Mar 24, 2021

View reviewed changes

MichaelMauderer added 2 commits March 24, 2021 13:25

fixup! feat: Show visualization on output port hover.

24345e2

fixup! doc: Add changelog entry.

99397a4

MichaelMauderer force-pushed the wip/mm/ide-1302-show-visualization-on-output-port-hover branch from c9e150a to 99397a4 Compare March 24, 2021 13:01

Merge branch 'develop' into wip/mm/ide-1302-show-visualization-on-out…

f71f309

…put-port-hover

MichaelMauderer added 3 commits March 25, 2021 12:59

Merge branch 'develop' into wip/mm/ide-1302-show-visualization-on-out…

07baa1c

…put-port-hover

fix: Correctly calculate padding for labels.

7f798fe

refactor: Change placement of output port tooltips to be below cursor.

2cd9c37

mwu-tow self-requested a review March 25, 2021 16:16

mwu-tow suggested changes Mar 25, 2021

View reviewed changes

MichaelMauderer added 2 commits March 27, 2021 18:05

fixup! feat: Show visualization on output port hover.

5dfb09f

Merge branch 'develop' into wip/mm/ide-1302-show-visualization-on-out…

a2a305c

…put-port-hover

MichaelMauderer requested a review from mwu-tow March 29, 2021 09:02

mwu-tow suggested changes Mar 30, 2021

View reviewed changes

Merge branch 'develop' into wip/mm/ide-1302-show-visualization-on-out…

08b8b62

…put-port-hover

fix: Only show preview vis when hovering output port, not whole node.

7198be2

MichaelMauderer added 2 commits March 30, 2021 15:13

Merge branch 'develop' into wip/mm/ide-1302-show-visualization-on-out…

5daa1da

…put-port-hover # Conflicts: # CHANGELOG.md # src/rust/ide/view/graph-editor/src/component/visualization/container.rs # src/rust/ide/view/graph-editor/src/lib.rs

fix: Correct ordering of layers.

51ca731

MichaelMauderer requested a review from mwu-tow March 30, 2021 13:38

farmaazon and others added 2 commits March 30, 2021 16:43

Merge branch 'develop' into wip/mm/ide-1302-show-visualization-on-out…

bc9fb79

…put-port-hover

Merge branch 'develop' into wip/mm/ide-1302-show-visualization-on-out…

8b12b3e

…put-port-hover

mwu-tow approved these changes Mar 30, 2021

View reviewed changes

MichaelMauderer and others added 2 commits March 30, 2021 19:33

[ci build]

a66f4d9

[ci build]

Merge branch 'develop' into wip/mm/ide-1302-show-visualization-on-out…

59fba43

…put-port-hover

farmaazon force-pushed the wip/mm/ide-1302-show-visualization-on-output-port-hover branch from 1e1e044 to 59fba43 Compare March 30, 2021 18:50

farmaazon merged commit b869971 into develop Mar 30, 2021

farmaazon deleted the wip/mm/ide-1302-show-visualization-on-output-port-hover branch March 30, 2021 18:51

mwu-tow pushed a commit to enso-org/enso that referenced this pull request Oct 30, 2021

Show visualization on output port hover. (enso-org/ide#1363)

8767c7b

Original commit: enso-org/ide@b869971

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Show visualization on output port hover. #1363

Show visualization on output port hover. #1363

MichaelMauderer commented Mar 23, 2021 •

edited

Loading

wdanilo left a comment

wdanilo Mar 24, 2021

wdanilo commented Mar 24, 2021

MichaelMauderer commented Mar 25, 2021

mwu-tow left a comment •

edited

Loading

farmaazon commented Mar 26, 2021

BASIC SCENARIO

TRAMS SCENARIO

MichaelMauderer commented Mar 29, 2021 •

edited

Loading

MichaelMauderer commented Mar 29, 2021

TRAMS SCENARIO

farmaazon commented Mar 29, 2021

mwu-tow left a comment •

edited

Loading

MichaelMauderer commented Mar 30, 2021 •

edited

Loading

mwu-tow commented Mar 30, 2021

MichaelMauderer commented Mar 30, 2021

mwu-tow commented Mar 30, 2021

		frp.source.on_reset <+ transition.value.map(\|t\| (t - 0.0).abs() < std::f32::EPSILON).on_true();
		frp.source.on_end <+ transition.value.map(\|t\| (t - 1.0).abs() < std::f32::EPSILON).on_true();

Show visualization on output port hover. #1363

Show visualization on output port hover. #1363

Conversation

MichaelMauderer commented Mar 23, 2021 • edited Loading

Pull Request Description

Checklist

wdanilo left a comment

Choose a reason for hiding this comment

wdanilo Mar 24, 2021

Choose a reason for hiding this comment

wdanilo commented Mar 24, 2021

MichaelMauderer commented Mar 25, 2021

mwu-tow left a comment • edited Loading

Choose a reason for hiding this comment

BASIC SCENARIO

TRAMS SCENARIO

farmaazon commented Mar 26, 2021

BASIC SCENARIO

TRAMS SCENARIO

MichaelMauderer commented Mar 29, 2021 • edited Loading

MichaelMauderer commented Mar 29, 2021

TRAMS SCENARIO

farmaazon commented Mar 29, 2021

mwu-tow left a comment • edited Loading

Choose a reason for hiding this comment

MichaelMauderer commented Mar 30, 2021 • edited Loading

mwu-tow commented Mar 30, 2021

MichaelMauderer commented Mar 30, 2021

mwu-tow commented Mar 30, 2021

MichaelMauderer commented Mar 23, 2021 •

edited

Loading

mwu-tow left a comment •

edited

Loading

MichaelMauderer commented Mar 29, 2021 •

edited

Loading

mwu-tow left a comment •

edited

Loading

MichaelMauderer commented Mar 30, 2021 •

edited

Loading