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

Component list panel integration #3530

Merged

Conversation

farmaazon
Copy link
Contributor

@farmaazon farmaazon commented Jun 15, 2022

Pull Request Description

This PR contains all work for finishing integration of first Component List Panel in the IDE:

  • It adds a stub for the whole Component Browser View. The documentation panel is re-used from the old searcher.
  • It has the presenter implementation, integrating the view with Hierarchical Component List from the controller.
  • It extends the View API, so the integration is possible, making use of Component Group Set wrapper.
  • The selection integration was also merged into this PR, because it depended on the API extension mentioned above. However, we should avoid such practice in the future.
cb.mp4

Important Notes

There are some known issues, to-be-fixed in the future.

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 dist and ./run ide watch.

@farmaazon farmaazon force-pushed the wip/farmaazon/component-list-panel-integration-181869402 branch 2 times, most recently from 8ea319e to deebfc3 Compare June 17, 2022 11:04
@farmaazon farmaazon force-pushed the wip/farmaazon/component-list-panel-integration-181869402 branch 2 times, most recently from 39d0bcc to f134bcf Compare June 27, 2022 10:17
@farmaazon farmaazon force-pushed the wip/farmaazon/component-list-panel-integration-181869402 branch from 47e6c3e to 05102d0 Compare July 1, 2022 12:34
farmaazon and others added 8 commits July 4, 2022 16:59
Proper positioning

Managing group events

Selecting things with Tab

Documentation

Fix documentation

Add some docs

Fix navigator

Fix compilation after rebase

favorites integration

Fix warnings and format code

Filtering WIP

Fix positioning

Fixes after rebase

Fix filtering
Fix layers ordering for scrollbars

Adjust selection sizes & implement corners radius animation

Try to fix camera
@farmaazon farmaazon force-pushed the wip/farmaazon/component-list-panel-integration-181869402 branch from bd4f196 to 84e58b8 Compare July 4, 2022 15:03
@farmaazon farmaazon force-pushed the wip/farmaazon/component-list-panel-integration-181869402 branch from a6ce698 to 5196f49 Compare July 4, 2022 15:25
@farmaazon farmaazon self-assigned this Jul 5, 2022
@farmaazon farmaazon marked this pull request as ready for review July 6, 2022 15:10
Copy link
Contributor

@akavel akavel left a comment

Choose a reason for hiding this comment

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

The following list contains the first part of issues and doubts I observed when doing the Acc/QA Review of this PR. It contains the issues I found in initial testing and that I was able to reproduce deterministically. I plan to continue working on the review, by doing a further "deeper" round of review, comparing the PR's behavior with the Design Doc; in case it surfaces more issues, I plan to add them in another comment. During the initial review, I also observed some issues I was not able to reproduce deterministically yet; I will work on trying to reproduce them, and if failed, will discuss on Discord what we want to do with those.

This list covers behaviors related to 2 tasks: https://www.pivotaltracker.com/story/show/181869402 and https://www.pivotaltracker.com/story/show/182493600.

1. Warnings about views being not registered show in Developer Console.

Screenshot 2022-07-11 at 11 41 49

2. Selection does not follow mouse when scrolling and can show half-way between entries.

Notably, this makes it unclear which entry is "selected" when half-way between entries and what will happen if TAB or ENTER is pressed then - see the video below.

At one point, I got the selection to overlap with the group header (see the screenshot below), but could not reproduce this glitch again yet.

Screen.Recording.2022-07-11.at.12.54.52.mov

Screenshot 2022-07-11 at 11 47 33

3. Bad UX: In the Sub-Modules section, all groups have the same name "Standard.Base..."

This makes the group names practically useless for distinguishing what the groups contain. The Documentation panel does not show the name nor path of the modules either.

Screenshot 2022-07-11 at 11 50 29

4. Bad UX: Cannot see full name of components with long name.

The names get clipped in the Component Browser Panel. The Documentation panel sometimes shows the name (but not the path) for a component with long name, but sometimes does not show the name - see the screenshots below (looks like name is only shown if the component does not have documentation available).

Screenshot 2022-07-11 at 11 53 13

Screenshot 2022-07-11 at 11 50 49

5. Bad UX: Cannot scroll the Documentation panel when hovering an item in the middle column.

See the video below. Presumably, this will be fixed in https://www.pivotaltracker.com/story/show/180872763.

https://drive.google.com/file/d/103soS1-NWu0eBFBSPo0ZrtWIvDa1jlEk/view?usp=sharing

6. Selection can show over empty space in Local Scope section.

To reproduce:

  1. Hover the mouse over an item in the left column of the Local Scope section.
  2. Type letters "ab" on the keyboard.
  3. Observe that the selection shows over a space that does not contain any entry of the Component Browser.

Screenshot 2022-07-11 at 12 21 19

7. Inconsistent UX: The "No Entries" text in Local Scope section has a fixed width font.

All other text in the Component Browser is rendered with proportional font, so the use of a fixed width font in the Local Scope section looks inconsistent. (See the screenshot above, in issue 6.)

8. When Expression Input area is selected, its selection overlaps Component Browser.

Not sure if this is working as designed or not. Looks a bit weird to me.

Screenshot 2022-07-11 at 12 11 29

@farmaazon
Copy link
Contributor Author

1. Warnings about views being not registered show in Developer Console.

Done

2. Selection does not follow mouse when scrolling and can show half-way between entries.

Seems like a problem with the Ilya's part. As he is on vacation, I can look at it

3. Bad UX: In the Sub-Modules section, all groups have the same name "Standard.Base..."

I've applied a workaround, where we skip "Standard" part. In the future, the Standard should be a separate section (as each namespace).

4. Bad UX: Cannot see full name of components with long name.

This is one I don't know how to cope. Generally, I'm limited by the way libraries names their things. Perhaps the causten font will be able to present more letters. Or I can add a title to the documentation with full name (and type) - as it is in the entries without docs.

5. Bad UX: Cannot scroll the Documentation panel when hovering an item in the middle column.

Yes, let's wait for focus management.

6. Selection can show over empty space in Local Scope section.
7. Inconsistent UX: The "No Entries" text in Local Scope section has a fixed width font.

To be done.

8. When Expression Input area is selected, its selection overlaps Component Browser.

In this one I need help of @wdanilo - Generally, the expression input is in a different layer - so it can be under the Component Browser (and the CB's shadow is over the node) or over it, but then we have the issue with node selection.

Copy link
Contributor

@akavel akavel left a comment

Choose a reason for hiding this comment

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

See below for a new set of issues + followups on previous issues. I did this round of the review on commit b8e75ef54. The testing continues, so the list is not yet expected to be final.

Status summary.
SOLVED: 1, 5 (postponed), 7.
TODO: 2, 3 (see below), 4 (see below), 6, 8, (new - see below:) 9-16.

Re: 3. Most of the group headers look much better now. However, some of them are still clipped - see e.g.: "Visualization.G..." in the screenshot below. My personal suggestion for a (potentially temporary) workaround would be to show the un-clipped name in the Documentation panel.

Screenshot 2022-07-12 at 12 48 37

Re: 4. The full path is still not visible in the Documentation panel for the components which do not have documentation available. For example, see the screenshot below - I can only see that stdout seems to be a method of System Pro..., but don't know what exactly System Pro... is:

Screenshot 2022-07-12 at 13 36 36

9. Bad UX: The documentation panel hides the mouse cursor.

Screen.Recording.2022-07-12.at.10.50.24.mov

10. Pressing enter does not open the Component Browser.

The "Opening the component browser" section of the design doc describes that pressing enter should open the CB:

  • Pressing either the tab key or the enter key.

11. Sub-modules section does not show the Maybe type nor its methods.

Steps to reproduce:

  1. Create new project.
  2. Press tab to open the Component Browser.
  3. Type Maybe on the keyboard.
  4. Scroll the Component Browser to the Sub-Modules section.
    • observed: the Sub-Modules section is empty.
    • expected: the Sub-Modules section contains the Maybe module and the Maybe.Some constructor.
  5. Continue typing on the keyboard - type .Some 10 (full expression should read: Maybe.Some 10).
  6. Press enter key. (A new node is created containing: Maybe.Some 10 expression.)
  7. Press tab while the new node is selected.
  8. Type is_ on the keyboard.
  9. Scroll the Component Browser to the Sub-Modules section.
    • observed: the Sub-Modules section is empty.
    • expected: the Sub-Modules section should contain the is_some method of the Maybe type.

Relevant fragment of the Design Doc - section "Language Support":

(...) given the expression node2 = node1.▯, all the methods of the type of node1 should be hinted to the user.

12. Component Browser does not show a sort method suggestion in the default project.

Steps to reproduce:

  1. Create a new project.
    • Observe that the project contains 2 nodes, the lower of which contains the operator1.sort expression.
  2. Drag an output connection from the 1st (upper) node and drop it on stage.
    • Observe that the Component Browser opens.
  3. Scroll the Component Browser to the Sub-Modules section.
    • observed: The section contains 2 groups, with the following entries:
      • Method filter_map of Vector
      • Method point_data of Vector
    • expected: The section should also contain a sort entry, to allow re-creating a copy of the 2nd node present in the default project.

13. Pressing cmd + enter does not accept the current expression "as is".

Steps to reproduce:

  1. Create a new project.
  2. Press tab to open the Component Browser.
  3. Type foobar.
  4. Press cmd + enter.
    • observed: nothing happens.
    • expected: the Component Browser is closed and a new node is created containing a foobar expression.

Relevant fragment of the design doc: design/design.md at main · enso-org/design

Pressing the Expression Acceptance Key while holding the command key or while the Expression Input Panel is focused, the user-provided input should be accepted "as is", even if it does not match any suggestion known to the Expression Search Engine.

Note: It appears possible to accept the expression by pressing enter alone, without holding the command key.

14. Pressing tab quickly followed by enter leaves a floating "No Entries." text in the IDE.

See the video below. Note that this happens in a newly loaded IDE; if the Component Browser was already opened before, the behavior may differ.

Screen.Recording.2022-07-12.at.13.16.08.mov

15. Bad UX: Opening the Component Browser 2nd or later time flashes its last contents from the previous time.

Steps to reproduce:

  1. Press tab to open the Component Browser.
  2. Scroll up the contents of the Component Browser.
  3. Press esc to close the Component Browser.
  4. Press tab to open the Component Browser.
    • observed: the contents of the CB from the moment before the esc key was pressed is momentarily flashed. See the video below, specifically observe the time span between 0:06 and 0:07.
    • expected: the contents of the Component Browser would start from a fully reset state, without flashing any earlier state.

Note: in some cases, the "earlier state" stays visible for much longer, but I'm not yet sure what are the specific required conditions.

Screen.Recording.2022-07-12.at.13.23.59.mov

16. Selecting another node does not accept current expression.

Steps to reproduce:

  1. Create a new project.
  2. Move the mouse to the right of the top node and open the Component Browser by pressing tab.
  3. Type asdf on the keyboard into the Component Browser.
  4. Move the mouse over the original top node and click it to select it.
    • observed: the top node becomes selected and the Component Browser is still visible.
    • expected: the top node becomes selected, the Component Browser is closed, and a new node with an expression asdf is created.

Relevant fragment from the design doc: design/design.md at main · enso-org/design

(...) selecting another node (...) should accept the currently focused suggestion before closing the Component Browser.

@farmaazon
Copy link
Contributor Author

farmaazon commented Jul 12, 2022

Re: 3. Most of the group headers look much better now. However, some of them are still clipped - see e.g.: "Visualization.G..." in the screenshot below. My personal suggestion for a (potentially temporary) workaround would be to show the un-clipped name in the Documentation panel.

I try to do that.

9. Bad UX: The documentation panel hides the mouse cursor.

This one is not easily fixable @wdanilo do you know when the "canvas cutting" will be implemented? BTW it is not clearly in the scope if this User Story - in this task I just re-use the old doc panel, which had this issue.

10. Pressing enter does not open the Component Browser.

The "Opening the component browser" section of the design doc describes that pressing enter should open the CB:

  • Pressing either the tab key or the enter key.

This is also not in the scope of the US, but I can check if it's easily addable.

11. Sub-modules section does not show the Maybe type nor its methods.
...
Relevant fragment of the Design Doc - section "Language Support":

This is for sure a part of another user story.

12. Component Browser does not show a sort method suggestion in the default project.

This is an issue on the Engine part. It is already on pivotal AFAIK, and should be mentioned in "Known issues" section.

13. Pressing cmd + enter does not accept the current expression "as is".

To be fixed

14. Pressing tab quickly followed by enter leaves a floating "No Entries." text in the IDE.

To be checked.

15. Bad UX: Opening the Component Browser 2nd or later time flashes its last contents from the previous time.

Should disappear once the performance will be improved.

16. Selecting another node does not accept current expression.

To be fixed

Copy link
Contributor

@akavel akavel left a comment

Choose a reason for hiding this comment

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

See below for the third set of issues.

17. Component Browser not shifted right when near left edge of the window.

The User Story links the "Placement of other Panels" section of the Design Doc as In Scope (link text: "the section about placement"). This section contains the following text fragment:

If there is not enough space in the application window, the Panels can be shifted right in such way, that the Component Browser List Panel and the Expression Input Panel will be left-aligned. There should be a minimal space kept between the left side of the window and the left side of the Component Browser:
image

This feature does not appear to work as described in the fragment above - see the screenshot below:

Screenshot 2022-07-12 at 16 34 31

18. Clicking and holding the mouse on the scroll-bar, outside the "trolley", scrolls the scroll-bar only once.

The following fragment is part of the "scrolling" section of the Design Doc, which is part of the "Component List Panel" section (linked as "the main section about panel" in the User Story):

Pressing and holding mouse [above or under the scrollbar] should repeat this [scrolling] event with a hardcoded rate after a hardcoded time.

However, pressing and holding mouse above or under the scrollbar appears to result only in one scroll event - see the video below:

Screen.Recording.2022-07-12.at.16.40.39.mov

19. The "No Entries." text is too small and wrongly placed when zoomed out.

Steps to reproduce:

  1. Create a new project.
  2. Select the top node.
  3. Zoom out.
  4. Press tab to open the Component Browser.
    • observed: The "No Entries." text is not visible in the Local Scope section; instead, it is located at incorrect location on screen and zoomed out. See the screenshot below. Note that zooming-out and zooming-in while the Component Browser is open also influences the size and position of the "No Entries." text.
    • expected: Correctly sized text should be visible in the Local Scope section of the Component Browser.

Screenshot 2022-07-12 at 16 48 55

20. Custom colors of virtual component groups are not used.

Steps to reproduce:

  1. Create a new project and save it on disk.

  2. Close the IDE. (Make sure that the Project Manager is also closed.)

  3. Edit the package.yaml file of the newly created project to add the following contents:

    component-groups:
      new:
        - Mcdbg Group 1:
            color: '#000000'
            exports:
              - Standard.Base.Data.Time
              - Standard.Table.Data.Table.Table
              - Standard.Base.System.File.new
        - Mcdbg Group 2:
            color: '#FF00FF'
            exports:
              - Standard.Base.Data.Time
              - Standard.Table.Data.Table.Table
              - Standard.Base.System.File.new
        - Mcdbg Group 3:
            color: '#FF00FF'
            exports:
              - Standard.Base.Data.Time
              - Standard.Table.Data.Table.Table
              - Standard.Base.System.File.new
    
  4. Open the IDE again and load the edited project.

  5. Press tab to open the Component Browser.

  6. Scroll up the Component Browser to see the Mcdbg groups in the "Favorite Data Science Tools" section.

    • observed: The groups have different colors than the ones specified in the package.yaml file. See the screenshots below.
    • expected: The Mcdbg Group 1 should have black color. The Mcdbg Group 2 and Mcdbg Group 3 should both have magenta color.

Screenshot 2022-07-12 at 17 51 45 Screenshot 2022-07-12 at 17 51 53 Screenshot 2022-07-12 at 17 52 02

21. Scrolling while the mouse hovers right-most column of Component Browser results in whole graph panning.

Steps to reproduce:

  1. Create a new project.
  2. Select the bottom node.
  3. Press tab to open the Component Browser.
  4. Using the mouse (or touchbar), position the cursor over the right-most column of the Component Browser.
  5. Scroll using the "two-finger gesture" on a Mac touchbar (may possibly reproduce using the mouse scroll wheel as well):
    • observed: The whole Graph Editor is panned in addition to the Component Browser contents scrolling. See the video below.
    • expected: Only the contents of the Component Browser should scroll; the Graph Editor should not pan.
Screen.Recording.2022-07-12.at.18.10.23.mov

@farmaazon
Copy link
Contributor Author

We decided (me and @akavel) to merge this branch despite the issues, because:

  1. The new Component Browser is under feature flag, turned off by default
  2. The PR does not cause regressions in the old searcher
  3. The PR is blocking @akavel's work

The issues will be reported in a separate tasks, and planned soon.

@farmaazon farmaazon added the CI: Ready to merge This PR is eligible for automatic merge label Jul 13, 2022
@mergify mergify bot merged commit b0d627a into develop Jul 14, 2022
@mergify mergify bot deleted the wip/farmaazon/component-list-panel-integration-181869402 branch July 14, 2022 12:00
@akavel
Copy link
Contributor

akavel commented Jul 18, 2022

QA Review status update:

  1. Fixed.
  2. Postponed: https://www.pivotaltracker.com/story/show/182713338
  3. Partially fixed. Remainder postponed: https://www.pivotaltracker.com/story/show/182713407
  4. Fixed.
  5. Postponed/not in scope: https://www.pivotaltracker.com/story/show/180872763
  6. Postponed: https://www.pivotaltracker.com/story/show/182748789
  7. Fixed.
  8. Postponed: https://www.pivotaltracker.com/story/show/182749100
  9. Not in scope.
  10. Not in scope.
  11. Not in scope. See: https://www.pivotaltracker.com/story/show/182573549.
  12. Not in scope. See: https://www.pivotaltracker.com/story/show/182573549.
  13. Postponed: https://www.pivotaltracker.com/story/show/182713510
  14. Postponed: https://www.pivotaltracker.com/story/show/182713432
  15. Expected to be fixed by: https://www.pivotaltracker.com/story/show/182610422
  16. Postponed: https://www.pivotaltracker.com/story/show/182713510
  17. Postponed: https://www.pivotaltracker.com/story/show/181870555
  18. Postponed: https://www.pivotaltracker.com/story/show/182713594
  19. Postponed: https://www.pivotaltracker.com/story/show/182713626
  20. Postponed: https://www.pivotaltracker.com/story/show/182720354
  21. Postponed: https://www.pivotaltracker.com/story/show/182713670

mergify bot pushed a commit that referenced this pull request Jul 21, 2022
)

Show default icons for all entries in the Component Browser. The icons are assigned to each entry depending on its kind.

https://www.pivotaltracker.com/story/show/182584326

#### Visuals

See below for a video showing entries of 5 different kinds in the Component Browser, each having a different icon. When watching the video, please note that the following are preexisting, known issues, not introduced by this PR:
- Selection is misaligned when hovering the mouse over the "new" component in the "Mcdbg Group 1" group - reported as issue 2 [in comments to PR 3530](#3530 (review)).
- [Names of Modules and Atoms displayed in Component Browser start with a small letter.](https://www.pivotaltracker.com/story/show/182745386)




https://user-images.githubusercontent.com/273837/179016109-c3ebab5a-0205-4b44-85b8-df3129edd75d.mov

# Important Notes
- A new derive macro `ForEachVariant` is defined and added to the `enso-prelude` crate.
mergify bot pushed a commit that referenced this pull request Jul 22, 2022
Filter the Virtual Component Groups (a.k.a. "Favorites Data Science Tools") in the `component::List` (a.k.a. Hierarchical Action List) to only contain components with IDs listed in the Engine's response to a `search/completion` request.

This completes the "Virtual Component Groups filtered by input type" task (linked below) because the Engine's response to a `search/completion` request contains IDs of components filtered by input node type and `this` type.

https://www.pivotaltracker.com/story/show/182661634

#### Visuals

See below for a video showing the list of Favorites filtered by the type of the input node. Please note that the video also displays a few known issues that are present in the existing code and not introduced by this PR:
- "Opening the Component Browser 2nd or later time flashes its last contents from the previous time" - reported as [issue 15 in PR 3530](#3530 (review)).
- The text of all the entries in the Component Browser does not show immediately, but the entries appear one by one instead (this is related to the performance of the current implementation of Component Browser and Text Area).

https://user-images.githubusercontent.com/273837/179000801-65ee7388-dde6-44b9-90fb-7453b4fb788c.mov


A screenshot showing the default, unfiltered list of Favorites when no input node is selected:

<img width="440" alt="Screenshot 2022-07-14 at 15 58 26" src="https://user-images.githubusercontent.com/273837/179000404-f14773a3-35a9-4e7a-877d-fcbb477b4769.png">
@akavel akavel mentioned this pull request Aug 9, 2022
4 tasks
mergify bot pushed a commit that referenced this pull request Aug 17, 2022
Add "text input" and "number input" virtual entries in the "Input" virtual component group in the Component Browser. The entries provide an easy way to put strings and numbers into a graph.

https://www.pivotaltracker.com/story/show/181870589

#### Visuals

See below for a video showing the "text input" and "number input" virtual entries in the "Input" component group in the "Favorites Data Science Tools" section of the Component Browser. Please note that the video also displays a few known issues that are present in the existing code and not introduced by this PR:

- "Opening the Component Browser 2nd or later time flashes its last contents from the previous time" - reported as [issue 15 in PR 3530](#3530 (review)) (which is [expected](#3530 (comment)) to be fixed by https://www.pivotaltracker.com/story/show/182610422).
- The text of all the entries in the Component Browser does not show immediately, but the entries appear one by one instead (this is related to the performance of the current implementation of Component Browser and Text Area).
- Selection in the Component Browser can show half-way between entries - reported as https://www.pivotaltracker.com/story/show/182713338.

https://user-images.githubusercontent.com/273837/183472391-c14eeded-481f-492e-a1b8-b86f42faf0cd.mov

# Important Notes
- The virtual entries are not filtered by input type or return type. The filtering is expected to be implemented in https://www.pivotaltracker.com/story/show/182842444.
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.

5 participants