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.

Sign up for GitHub

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

Jump to bottom

Dynamic dropdown support #4072

Merged
merged 30 commits into from
Feb 4, 2023
Merged

Dynamic dropdown support #4072

merged 30 commits into from
Feb 4, 2023

Conversation

Frizi
Copy link
Contributor

@Frizi Frizi commented Jan 23, 2023 •
edited
Loading

Pull Request Description

Implementation of https://www.pivotaltracker.com/story/show/184012743

column-name-widget-switching.mp4

This is also a step towards more general widget support. The widget metadata is queried using Meta.get_annotation method through a dedicated visualization. For now only Single_Choice case is handled, and always all suggestions are is returned.

Important Notes

There are limitations as to which node segments receive a widget. Only chain method calls are supported now (thing.method syntax), and only outside of lambda scope. Widgets in lambdas will require support for visualisations of lambda subexpressions, which is currently missing in the engine. The IDE technically tries to place the widgets there, but the data never arrives. It should work once the engine support is added.

This PR includes a mock for Meta.get_annotation call that only supports Table.at method. Real implementation is a separate task that is already in progress.

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.

@Frizi Frizi force-pushed the wip/frizi/dynamic-dropdown-184012743 branch 2 times, most recently from a58934a to ddce482 Compare January 25, 2023 20:22
@Frizi Frizi marked this pull request as ready for review January 26, 2023 21:03
wdanilo
wdanilo requested changes Jan 26, 2023
View reviewed changes
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.

Also, the video does not work:

image

CHANGELOG.md Outdated Show resolved Hide resolved
app/gui/language/ast/impl/src/opr.rs Outdated Show resolved Hide resolved
app/gui/language/span-tree/src/generate.rs Outdated Show resolved Hide resolved
app/gui/language/span-tree/src/generate.rs Outdated Show resolved Hide resolved
app/gui/language/span-tree/src/generate.rs Outdated Show resolved Hide resolved
app/gui/view/graph-editor/src/component/node/input/widget.rs Outdated Show resolved Hide resolved
app/gui/view/graph-editor/src/component/node/input/widget.rs Outdated Show resolved Hide resolved
app/gui/view/graph-editor/src/component/node/input/widget.rs Outdated Show resolved Hide resolved
app/gui/view/graph-editor/src/component/node/input/widget.rs Outdated Show resolved Hide resolved
app/gui/view/graph-editor/src/component/node/input/widget.rs Outdated Show resolved Hide resolved
@Frizi Frizi force-pushed the wip/frizi/dynamic-dropdown-184012743 branch 2 times, most recently from eade623 to 600d0ef Compare February 1, 2023 12:37
@Frizi Frizi requested a review from wdanilo February 1, 2023 12:42
@Frizi Frizi force-pushed the wip/frizi/dynamic-dropdown-184012743 branch from 600d0ef to 7cc15d4 Compare February 1, 2023 14:48
wdanilo
wdanilo approved these changes Feb 2, 2023
View reviewed changes
app/gui/language/span-tree/src/generate.rs Outdated Show resolved Hide resolved
app/gui/language/span-tree/src/generate.rs Outdated Show resolved Hide resolved
app/gui/language/span-tree/src/generate.rs Outdated Show resolved Hide resolved
app/gui/language/span-tree/src/generate.rs Outdated Show resolved Hide resolved
app/gui/language/span-tree/src/generate/context.rs Outdated Show resolved Hide resolved
app/gui/src/controller/graph/widgets.rs Outdated Show resolved Hide resolved
app/gui/src/controller/graph/widgets.rs Outdated Show resolved Hide resolved
@Frizi Frizi added the CI: Ready to merge This PR is eligible for automatic merge label Feb 2, 2023
@Frizi Frizi force-pushed the wip/frizi/dynamic-dropdown-184012743 branch from 8e6e02b to ee12641 Compare February 3, 2023 09:19
@Frizi Frizi added CI: Clean build required CI runners will be cleaned before and after this PR is built. CI: Ready to merge This PR is eligible for automatic merge and removed CI: Clean build required CI runners will be cleaned before and after this PR is built. CI: Ready to merge This PR is eligible for automatic merge labels Feb 3, 2023
@Frizi Frizi force-pushed the wip/frizi/dynamic-dropdown-184012743 branch from ca3ed07 to 660ad12 Compare February 3, 2023 11:27
@Frizi Frizi added the CI: Clean build required CI runners will be cleaned before and after this PR is built. label Feb 3, 2023
@Frizi Frizi force-pushed the wip/frizi/dynamic-dropdown-184012743 branch 3 times, most recently from 0277bbc to a0bdd5d Compare February 3, 2023 14:48
@Frizi
use Set dirty flag for RemovedChildren for 5x faster dropdown openening
56a2a53
@Frizi
cleanup widget data deserialization
d5f0b05
@Frizi
hide widgets during node editing and fix another memory leak in unini…
710f408 
…t dropdown
@Frizi
do not store any network reference in lazy dropdown
98de98c
@Frizi
improve span tree argument info generation, fix tests and lint issues
fedf066
@Frizi
added changelog entry
06a4611
@Frizi
improve doc comment wording
f011c3a
@Frizi
refactor ApplicationBase
d8d4715
@Frizi
refactor widget registration to only use call_id, remove target_id fr…
e744a67 
…om argument info
@Frizi
fix tests and lints
1e93152
@Frizi
refactor widgets controller
7373218
@Frizi
improve names and comments, remove unnecessary ModelCommon
3942de6
@Frizi
improve request docs
9ec00d3
@Frizi
add is_function_parameter to span-tree node kind
c7aa654
@Frizi
remove Unset widget kind in favour of using Option
c2830e3
@Frizi
move port widget initialization to separate function
16f5ee9
@Frizi
lint
9b28ea0
@Frizi
singular widget module name and uppercase ID
558cdf6
@Frizi
fix docs
b922de4
@Frizi
fmt
d7d6025
@Frizi Frizi force-pushed the wip/frizi/dynamic-dropdown-184012743 branch from a0bdd5d to d7d6025 Compare February 3, 2023 14:49
hubertp
hubertp approved these changes Feb 3, 2023
View reviewed changes
Copy link
Contributor

@hubertp hubertp left a comment

Choose a reason for hiding this comment

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

Added a workaround for spurious failures

@hubertp
Mark tests as flaky until test cleanup is fixed
16ba57d 
There is a cleanup race-condition between individual tests casued by
asynchronous initialization of JGit. It's rare but happens and leads to
spurious failures.
Marking as flaky to unblock unrelated PRs.
@mergify mergify bot merged commit 6b14ec5 into develop Feb 4, 2023
@mergify mergify bot deleted the wip/frizi/dynamic-dropdown-184012743 branch February 4, 2023 00:50
@wdanilo wdanilo mentioned this pull request Mar 29, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wdanilo wdanilo wdanilo approved these changes

@hubertp hubertp hubertp approved these changes

@MichaelMauderer MichaelMauderer Awaiting requested review from MichaelMauderer

@farmaazon farmaazon Awaiting requested review from farmaazon farmaazon is a code owner

@mwu-tow mwu-tow Awaiting requested review from mwu-tow

@kazcw kazcw Awaiting requested review from kazcw kazcw is a code owner

@4e6 4e6 Awaiting requested review from 4e6 4e6 is a code owner

@JaroslavTulach JaroslavTulach Awaiting requested review from JaroslavTulach JaroslavTulach is a code owner

Assignees
No one assigned
Labels
CI: Clean build required CI runners will be cleaned before and after this PR is built. CI: Ready to merge This PR is eligible for automatic merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Frizi @hubertp @wdanilo