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

Fix missing visualization preview in CB #5757

Merged
merged 7 commits into from
Feb 27, 2023

Conversation

galin-enso
Copy link
Contributor

@galin-enso galin-enso commented Feb 23, 2023

Pull Request Description

Closes #5639

This PR fixes a regression introduced in #4120, due to which new nodes that were being edited had an empty visualization preview. Now newly created visualization containers do get a default visualization set even before the visualization's input type is set.

unknown_2023.02.24-19.05.mp4

Important Notes

Checklist

Please include the following checklist in your PR:

  • The documentation has been updated if necessary.
  • All code conforms to the
    Scala,
    Java,
    and
    Rust
    style guides.
  • All code has been tested:
    • Unit tests have been written where possible.
    • If GUI codebase was changed: Enso GUI was tested when built using BOTH
      ./run ide build and ./run ide watch.

@galin-enso galin-enso marked this pull request as ready for review February 24, 2023 18:14
@farmaazon farmaazon self-assigned this Feb 27, 2023
@vitvakatu
Copy link
Contributor

@farmaazon
I checked this PR. Preview visualization is visible now, though its behavior is a bit weird.

  1. I pressed enter, then edited the node by adding argument – it showed panic expected arg1 to be Function but got Function, which is a bit shizophrenic.
  2. I pressed tab and tried typing different arguments. The value in the preview did not change, even after some waiting. Moreover, some values produced the same panic, even though others didn't.
2023-02-27.17-46-22.mp4

This PR is not the cause though, as it happens on develop as well (except it does not show preview on develop).

The reason for such behavior is in our searcher implementation:

Screenshot 2023-02-27 at 18 13 02

We replace the expression with suggestion preview, and sometimes it is left in the code even though the the CB does not have any entries visible. Moreover, the previewed suggestion can be different at different times (perhaps it is caused by different ordering of filtered results when your enter different numbers). Otherwise the preview always shows the default value of index 0, and you can't see a preview of a different index.

@vitvakatu
Copy link
Contributor

QA: accepted, given the said issues are also reproducible on develop.

@farmaazon farmaazon added the CI: Ready to merge This PR is eligible for automatic merge label Feb 27, 2023
@mergify mergify bot merged commit b8ceaec into develop Feb 27, 2023
@mergify mergify bot deleted the wip/galin-enso/missing-cb-visualization_preview-5639 branch February 27, 2023 20:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: Ready to merge This PR is eligible for automatic merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Visualization Preview does not work
3 participants