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

#2266 Custom port images #2267

Merged
merged 14 commits into from
Dec 16, 2024
Merged

#2266 Custom port images #2267

merged 14 commits into from
Dec 16, 2024

Conversation

tomlyn
Copy link
Member

@tomlyn tomlyn commented Dec 10, 2024

Fixes: #2266

Developer's Certificate of Origin 1.1

   By making a contribution to this project, I certify that:

   (a) The contribution was created in whole or in part by me and I
       have the right to submit it under the Apache License 2.0; or

   (b) The contribution is based upon previous work that, to the best
       of my knowledge, is covered under an appropriate open source
       license and I have the right under that license to submit that
       work with modifications, whether created in whole or in part
       by me, under the same open source license (unless I am
       permitted to submit under a different license), as indicated
       in the file; or

   (c) The contribution was provided directly to me by some other
       person who certified (a), (b) or (c) and I have not modified
       it.

   (d) I understand and agree that this project and the contribution
       are public and that a record of the contribution (including all
       personal information I submit with it, including my sign-off) is
       maintained indefinitely and may be redistributed consistent with
       this project or the open source license(s) involved.

@tomlyn
elyra-ai#2266 Custom port images
088a9f1 
Signed-off-by: CTomlyn <[email protected]>
@tomlyn
New icon
15a4f93 
Signed-off-by: CTomlyn <[email protected]>
@tomlyn
Delete icon
b16b554 
Signed-off-by: CTomlyn <[email protected]>
@tomlyn
Fix angle of port image
c20f0b5 
Signed-off-by: CTomlyn <[email protected]>
@tomlyn
Refactor
3eaa533 
Signed-off-by: CTomlyn <[email protected]>
@tomlyn
Bracket
aec9d37 
Signed-off-by: CTomlyn <[email protected]>
@tomlyn
Image
7c5292d 
Signed-off-by: CTomlyn <[email protected]>
@tomlyn
Connect
e28c7ba 
Signed-off-by: CTomlyn <[email protected]>
@tomlyn tomlyn force-pushed the multi_port_images branch 3 times, most recently from e64768c to e28c7ba Compare December 13, 2024 02:19
@tomlyn
Fix errors 2
d4a0e71 
Signed-off-by: CTomlyn <[email protected]>
@tomlyn tomlyn requested a review from caritaou December 13, 2024 21:40
caritaou
caritaou reviewed Dec 13, 2024
View reviewed changes
canvas_modules/common-canvas/src/object-model/layout-dimensions.js Outdated
@@ -211,18 +233,28 @@ const portsHorizontalDefaultLayout = {
// this: { x_pos: 5, y_pos: 10, pos: "topRight" }. x_pos and y_pos are
// offsets from the pos point on the node.
// The order of the elements corresponds to the order of ports in the
// outputs array for the node.
// outputs array for the node. If there are more input ports than elements
Copy link
Member

Choose a reason for hiding this comment

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

typo, should be output

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch - fixed

canvas_modules/common-canvas/src/object-model/layout-dimensions.js Outdated
// { type: "jsx", src: (<FaceCool />), width: 16, height: 16 }
//
// The order of the elements corresponds to the order of ports in the
// inputs array for the node. If there are more input ports than elements
Copy link
Member

Choose a reason for hiding this comment

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

typo, should be output

Copy link
Member Author

Choose a reason for hiding this comment

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

Another good catch - fixed

canvas_modules/common-canvas/src/object-model/layout-dimensions.js Outdated
@@ -637,18 +691,28 @@ const portsVerticalDefaultLayout = {
// this: { x_pos: 5, y_pos: 10, pos: "topRight" }. x_pos and y_pos are
// offsets from the pos point on the node.
// The order of the elements corresponds to the order of ports in the
// outputs array for the node.
// outputs array for the node. If there are more input ports than elements
Copy link
Member

Choose a reason for hiding this comment

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

typo, should be output

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep good one - fixed

canvas_modules/common-canvas/src/object-model/layout-dimensions.js Outdated
// { type: "jsx", src: (<FaceCool />), width: 16, height: 16 }
//
// The order of the elements corresponds to the order of ports in the
// inputs array for the node. If there are more input ports than elements
Copy link
Member

Choose a reason for hiding this comment

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

typo, should be output

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

docs/pages/03.06.01-node-customization.md Outdated
// { type: "jsx", src: (<FaceCool />), width: 16, height: 16 }
//
// The order of the elements corresponds to the order of ports in the
// inputs array for the node. If there are more input ports than elements
Copy link
Member

Choose a reason for hiding this comment

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

typo, should be output

Copy link
Member Author

Choose a reason for hiding this comment

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

I copied and pasted the whole text from the layout-dimensions.js file so this is fixed

caritaou
caritaou reviewed Dec 13, 2024
View reviewed changes
Copy link
Member

@caritaou caritaou left a comment

Choose a reason for hiding this comment

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

I see an error on the console in the Network example when dragging a link out
image

@tomlyn
Copy link
Member Author

tomlyn commented Dec 15, 2024

I fixed the Network diagram problem -- thanks for trying that I had missed it. The code now sets default width and height for port images or guide images if they are missing from the application's node layout fields.

@tomlyn
Fixed review comments
e21d634 
Signed-off-by: CTomlyn <[email protected]>
@tomlyn
Enhancel sample app
5f2c730 
Signed-off-by: CTomlyn <[email protected]>
@tomlyn
Circle enhancments
71aef69 
Signed-off-by: CTomlyn <[email protected]>
@tomlyn
Docs
b491f92 
Signed-off-by: CTomlyn <[email protected]>
@tomlyn tomlyn requested a review from caritaou December 16, 2024 07:05
@tomlyn tomlyn merged commit 2c41ad5 into elyra-ai:main Dec 16, 2024
3 checks passed
@tomlyn tomlyn deleted the multi_port_images branch December 16, 2024 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@caritaou caritaou caritaou approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Custom port images
2 participants
@tomlyn @caritaou