-
Notifications
You must be signed in to change notification settings - Fork 12
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
💥Dropped link to add node and automatically connect its link via components panel #121
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the pull request!
This feature for sure be able to speed up developing xircuit workflows. From first try, looks like the first wheel is still a bit buggy when any component from the component panel is created. To reproduce:
- create component
- drag arrow -> component panel will pop up
- search component -> drop -> component will be created at 0, 0
I've also noted the log:
Uncaught TypeError: Cannot read properties of undefined (reading 'getOptions')
…ircuits into adry/components-panel
Thanks Fahreza for the review. It seems the problem occur when we built the wheel. It doesn't built the frontend when packaging the xircuits.
The workaround is I just add the labextension so that it can build the frontend when packaging. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome work, it feels much natural to use now!
There's 2 things that I'd like to comment on.
- The spawn location for the component bar. In smaller screens or if you drag it close to the edge, it'll clip left by a large margin.
- The component spawn location for right click. It goes to a location which I'm not very sure where it's based.
I think both can be solved if you can both spawn the menu and component at where the pointer is. I think the tricky part is how to translate the zoom ins and out coordinates, but it shouldn't drift too far out even if you don't have a handler for it. That, or hopefully RD has xy absolute coordinates.
Thanks for the feedback. Fixed both of the issues. Another thing I notice while fixing this is if you open the panel near to right side of screen, it'll go off screen. What do you think about this? Edit: Another things is the labextension folder. I don't think it's a good idea to have it in master. Because it'll generate many different files with each build so we've to push that change. I think I should remove it but need to make some change in github action to build the frontend when packaging.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome work, everything works smoothly now. Merged and thanks for the PR!
Description
This will enable to add node via context menu. It's also possible to automatically connect its link by dropping a link to canvas.
The components panel will only display what component it can linked but not smart enough to know 'Can it connect?' as there is no type check when the link is connecting.
There are 3 condition to display 3 different components list which are:
inPort
orOutport
will show all but General Components. (Can be added and linked)inPort
(beside ▶) will only display General Components. (Can be added and linked)Take note:
Dropping link from any parameter
OutPort
(beside ▶) will not display anything as it's hard to capture what it can link to.As this component panel can be activated by right-clicking, it make the previous Jupyterlab's context menu obsolete. With that, new custom context menu was created with fewer command mostly for nodes. It can be activated by ctrl+left-click.
Also, refactored and removed all the general components except literal and hyperparameter.
Pull Request Type
Type of Change
Tests
inPort
(beside ▶).Tested on?
Notes
Currently, there is no type check when connecting the link. Also, feels like some of the files in
/src/component
folder need to move to different folder.