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

Canvas: Add direction options for connections #84620

Merged
merged 8 commits into from
Mar 21, 2024

Conversation

drew08t
Copy link
Contributor

@drew08t drew08t commented Mar 15, 2024

Adding direction option for canvas connections.

For single segment connection:
Mar-15-2024 16-08-17

For multi-segment connection:
Mar-15-2024 16-04-28

Closes #83268

@drew08t drew08t added add to changelog area/panel/canvas Issues related to canvas panel no-backport Skip backport of PR labels Mar 15, 2024
@drew08t drew08t added this to the 11.0.x milestone Mar 15, 2024
@drew08t drew08t self-assigned this Mar 15, 2024
@drew08t drew08t marked this pull request as ready for review March 15, 2024 23:37
@drew08t drew08t requested a review from a team as a code owner March 15, 2024 23:37
@drew08t drew08t requested review from Develer and adela-almasan and removed request for a team March 15, 2024 23:37
Copy link
Contributor

@adela-almasan adela-almasan left a comment

Choose a reason for hiding this comment

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

Tested locally and it works good! It should be good to merge after minor cleanup! 🚀

@lukasztyrala
Copy link
Member

LGTM, we can work on the labels for selector (or using icons), but this is not something blocking this PR.

Copy link
Collaborator

@codeincarnate codeincarnate left a comment

Choose a reason for hiding this comment

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

Works as expected! Just had a few nits 😄

public/app/plugins/panel/canvas/editor/options.ts Outdated Show resolved Hide resolved
@@ -120,4 +121,22 @@ export const optionBuilder: OptionSuppliers = {
},
});
},

addDirection: (builder, context) => {
const category = ['Direction'];
Copy link
Collaborator

Choose a reason for hiding this comment

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

May make sense to call this "Connection Direction" or something like that. It's a bit ambiguous at the moment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think in this case, because it's in the context of a selected connection, I'll leave it as is for now to be consistent with its siblings.
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Or perhaps "Arrow Direction" would suffice?

@drew08t drew08t merged commit c1065e6 into main Mar 21, 2024
14 checks passed
@drew08t drew08t deleted the drew08t/canvas-connection-directions branch March 21, 2024 18:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
add to changelog area/frontend area/panel/canvas Issues related to canvas panel no-backport Skip backport of PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Canvas: Arrow head options
6 participants