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

Drop down values for take on Text should be shortened #6052

Closed
sylwiabr opened this issue Mar 22, 2023 · 2 comments · Fixed by #6878
Closed

Drop down values for take on Text should be shortened #6052

sylwiabr opened this issue Mar 22, 2023 · 2 comments · Fixed by #6878
Assignees
Labels
--bug Type: bug p-medium Should be completed in the next few sprints s-info-needed Status: more information needed from submitter

Comments

@sylwiabr
Copy link
Member

Discord username

No response

How important is this feature to you?

2 – Somewhat important, but I can live without it

Describe the idea you'd like to see implemented.

We a shortening drop down values for take on a Vector but for Text we are displaying FQN. It's hard to read (specially because longer names are cut).
image

Is your feature request related to a problem?

No response

Screenshots, screencasts, mockups.

No response

Would you be willing to help us implement this feature?

No

@vitvakatu
Copy link
Contributor

@Frizi am I correct that it is a bug? If I understand the idea correctly, every access chain should be shortened, so on the screenshot, even the Index_Sub_Range values should be much shorter.

@farmaazon farmaazon added g-widgets p-medium Should be completed in the next few sprints and removed triage labels Mar 31, 2023
@farmaazon
Copy link
Contributor

@Frizi bump

@farmaazon farmaazon added the s-info-needed Status: more information needed from submitter label May 2, 2023
@farmaazon farmaazon added this to the Design Partners milestone May 2, 2023
@sylwiabr sylwiabr moved this from ❓New to 📤 Backlog in Issues Board May 3, 2023
@vitvakatu vitvakatu moved this from 📤 Backlog to 🌟 Q/A review in Issues Board May 30, 2023
@mergify mergify bot closed this as completed in #6878 May 30, 2023
mergify bot pushed a commit that referenced this issue May 30, 2023
Implements #6792
Fixes #6715
Fixes #6052
Fixes #5689

The dynamic dropdown widgets entries now can specify additional widget configuration as a list of `parameters` of the inner method call. That allows for creating smarter widgets within nested constructors, taking the outer widget's context into account.

<img width="772" alt="image" src="https://github.com/enso-org/enso/assets/919491/97c70654-9170-4cf0-ae4d-2c25c74caa96">

With the changes to the serialization logic, I have also adressed issues related to automatic label generation for both static and dynamic dropdown entries. For access chains (e.g. `Foo.Bar.Baz_Qux`), the label will now always contain only the last segment, and all underscores will be removed (e.g. `Baz Qux`). This also applies to dynamic entries where the label is not explicitly specified in method annotation.

<img width="265" alt="image" src="https://github.com/enso-org/enso/assets/919491/1abe6c77-010b-4622-b252-97cd1543cb48">

Additionaly, now the dynamic entries containing constructors will also be resolved within suggestion database, allowing us to automatically insert relevant import, shorten the actually used expression and wrap it with parentheses if required. That was required for nested widgets to show up, as we depend on properly resolved argument names to show them. The widget definitions in annotations no longer need to wrap the expressions manually. Instead, the constructors used in dropdown entries should be specified using fully qualified names, similarly to how we do it in tag values.

CC @jdunkerley - The dropdown entries containing just a constructor will no longer need added parentheses around them. Instead, the constructors should be specified using fully qualified names, similarly to how we do it in tag values.

<img width="389" alt="image" src="https://github.com/enso-org/enso/assets/919491/19944b5b-d0c7-43ac-bf17-ca1556e0b3f0">

Note that currently the import resolution is attempted even if the used constructor is is not specified using a fully qualified name. To accomplish that, the IDE is performing a more expensive search through whole suggestion database for matching type and module (e.g. in example above, we are searching for a match for `Aggregate_Column.First`). If there are multiple potential matches due to a name collision, it is undefined which one would be preferred. Effectively one will be picked at random. To avoid that, the libraries should over time transition to using fully qualified names wherever possible.

# Important Notes
I have removed the `payload` field from the span tree, and with it the generic argument on its nodes. This was already partially done on the branch with new design, on which I also had a few changes that turned out to be useful for this PR. So I pulled it in as well. It is a nice simplification that will ease our further work on removing the span-tree altogether. The biggest impact it had was on the node output port, where I had to store the port data outside of the span tree. This is the approach we would be taking when transitioning to AST anyway.
@github-project-automation github-project-automation bot moved this from 🌟 Q/A review to 🟢 Accepted in Issues Board May 30, 2023
@farmaazon farmaazon moved this from 🟢 Accepted to 🗄️ Archived in Issues Board Jun 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
--bug Type: bug p-medium Should be completed in the next few sprints s-info-needed Status: more information needed from submitter
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants