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

kie-issue#206: Enable autocomplete suggestions for Literal Expressions and Decision Table input cells on DMN Editor's Boxed Expression Editor #2018

Merged
merged 3 commits into from
Oct 25, 2023

Conversation

danielzhe
Copy link
Contributor

Closes: apache/incubator-kie-issues#206

Enables autocomplete for FEEL expressions.

Screen.Recording.2023-10-18.at.15.52.29.mov

@danielzhe danielzhe temporarily deployed to ci October 19, 2023 18:59 — with GitHub Actions Inactive
@danielzhe danielzhe temporarily deployed to ci October 19, 2023 18:59 — with GitHub Actions Inactive
@danielzhe danielzhe added pr: waiting-for-review Waiting for peer reviews area:dmn labels Oct 19, 2023
@danielzhe danielzhe temporarily deployed to ci October 20, 2023 07:00 — with GitHub Actions Inactive
@danielzhe danielzhe temporarily deployed to ci October 20, 2023 07:00 — with GitHub Actions Inactive
@danielzhe danielzhe temporarily deployed to ci October 20, 2023 07:00 — with GitHub Actions Inactive
Copy link
Contributor

@jomarko jomarko left a comment

Choose a reason for hiding this comment

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

Thank you for this cool feature @danielzhe . I found some interesting scenario when monaco starts to offer fields of nested structures, I think it offered me inappropriate options on the second level. See :
Screenshot 2023-10-20 131325

and try to model same decision in:
types.dmn.txt

Also, am I correct imported models are not partof this PR, right? apache/incubator-kie-issues#539

@danielzhe
Copy link
Contributor Author

Thank you, @jomarko !
That options that Monaco suggest by itself when it doesn't have any valid options available, so it tries to "discover" some options that "maybe is ok". I don't know if we can disable this feature.

About the included models, they are not part of this PR yet. It's gonna take a few more time to make that feature available because we have to "parse" all the imported nodes and we don't want to load all models using the old Marshaller in GWT side because of performance reasons and it can lead to a lot of wasted work as soon as we finish the migration to a full non-GWT editor. @tiagobento is working on that.

…ressions and Decision Table input cells on DMN Editor's Boxed Expression Editor
@danielzhe danielzhe temporarily deployed to ci October 20, 2023 18:58 — with GitHub Actions Inactive
@danielzhe danielzhe temporarily deployed to ci October 20, 2023 18:58 — with GitHub Actions Inactive
@tiagobento
Copy link
Contributor

@danielzhe There is a way to disable these suggestions from Monaco. I did it for SWF, but I don't remember how :D

@danielzhe danielzhe temporarily deployed to ci October 23, 2023 06:27 — with GitHub Actions Inactive
@danielzhe danielzhe temporarily deployed to ci October 23, 2023 06:27 — with GitHub Actions Inactive
@danielzhe danielzhe temporarily deployed to ci October 23, 2023 06:27 — with GitHub Actions Inactive
@danielzhe danielzhe temporarily deployed to ci October 23, 2023 20:29 — with GitHub Actions Inactive
@danielzhe danielzhe temporarily deployed to ci October 23, 2023 20:29 — with GitHub Actions Inactive
@danielzhe danielzhe temporarily deployed to ci October 23, 2023 20:29 — with GitHub Actions Inactive
@danielzhe
Copy link
Contributor Author

@tiagobento Fixed the issue about recursive calls that we talked in pvt.

@danielzhe danielzhe temporarily deployed to ci October 24, 2023 18:38 — with GitHub Actions Inactive
@danielzhe danielzhe temporarily deployed to ci October 24, 2023 18:38 — with GitHub Actions Inactive
@danielzhe
Copy link
Contributor Author

@jomarko @tiagobento Changes applied, ready for your review. I also did an improvement to allow space to trigger the autocomplete suggestions.

@@ -83,6 +83,7 @@ export const feelDefaultConfig = (
minimap: {
enabled: false,
},
wordBasedSuggestions: false,
Copy link
Contributor

Choose a reason for hiding this comment

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

That's the one!

@danielzhe danielzhe temporarily deployed to ci October 24, 2023 22:09 — with GitHub Actions Inactive
@danielzhe danielzhe temporarily deployed to ci October 24, 2023 22:09 — with GitHub Actions Inactive
@danielzhe danielzhe temporarily deployed to ci October 24, 2023 22:09 — with GitHub Actions Inactive
Copy link
Contributor

@yesamer yesamer left a comment

Choose a reason for hiding this comment

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

LGTM, thank you @danielzhe for this amazing feature.
Please consider implementing a Test Suite for this new feature.

Copy link
Contributor

@jomarko jomarko left a comment

Choose a reason for hiding this comment

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

I can confirm, in situation described originally as https://github.com/kiegroup/kie-tools/pull/2018#pullrequestreview-1689835344 are not these 'wrong' autosuggestions present anymore.

In the same scenario, currently no suggestions are present, what is probably expected? At least that is my feelingreading comments from @danielzhe and @tiagobento

Screenshot 2023-10-25 141819

Thank you for the PR!

@yesamer yesamer merged commit e64948e into apache:main Oct 25, 2023
8 checks passed
@danielzhe danielzhe deleted the feel-autocomplete branch March 29, 2024 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:dmn pr: waiting-for-review Waiting for peer reviews
Projects
None yet
4 participants