Skip to content

Conversation

@parthbhatt268
Copy link

@parthbhatt268 parthbhatt268 commented Mar 31, 2025

Checklist

Ticket - https://sirensolutions.atlassian.net/browse/INVE-26256

  • npm test passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Description of change

In this PR,
We introduces a functionality to compute a custom label and direction when we group nodes with edges having identical labels and directions.

Rules followed :

  1. Identical Labels and Directions

    • Resultant VEdge gets <label> (count) (e.g., <some-label> (2)) and retains the unanimous direction.
  2. Identical Labels, Different Directions

    • Resultant VEdge gets <label> (count) (e.g., <some-label> (2)) but no direction.
  3. Different Labels and Directions

    • Resultant VEdge has a blank label and no direction (default).

How to Test

  1. Clone the G6 repository locally.
  2. Check out the branch: INVE-26256.
  3. Set up and connect kibi-internal with G6 by following the documentation. This ensures kibi-internal uses the local G6 package with the PR changes.
  4. Test the scenarios shown in the attached video.

Videos

Screencast.from.31-03-25.13.22.20.webm
Screencast.from.31-03-25.13.20.18.webm

@parthbhatt268 parthbhatt268 changed the title add logic to have custom label on vedge [INVE-26256] - Add logic to have custom label on vedge Mar 31, 2025
Copy link

@Blakko Blakko left a comment

Choose a reason for hiding this comment

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

Did a really quick pass
The number on the edge and the label inheritance need to be opt-in parameters. Meaning that the current behavior does not change, unless you set these flags to true.

Something like:

public collapseCombo(combo: string | ICombo, stack: boolean = true, opts: {
inheritLabel: boolean,
showCount: boolean
}): void {

@parthbhatt268
Copy link
Author

parthbhatt268 commented Apr 1, 2025

Hi Fabio,

  • I isolated the current code behind a flag (opts) to separate the old and new logic into distinct blocks (using if...else). I'm not 100% sure how to test this functionality effectively. I tried passing the opts param from Kibi-internal but am uncertain if Graphin interferes or handles the opts in between.
  • For now, the functionality remains hidden behind the opts flag, as visible in the code (if you want, we can default the feature to False instead of default True).
  • If there is anything else pending, please do let me know
    Thanks

@parthbhatt268 parthbhatt268 requested a review from Blakko April 2, 2025 09:17
@parthbhatt268 parthbhatt268 requested review from Blakko and szydan April 4, 2025 08:37
Copy link

@Blakko Blakko left a comment

Choose a reason for hiding this comment

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

To properly support this functionality in investigate, you'll need to update:

  • the collapseExpandCombo function in the core package

Additionally, if we want to get this merged we need to update

  • the collapse-expand-combo behavior in the pc package

And review all the places where the collapseExpandCombo, collapseCombo and expandCombo are used to see if we need to update them to have a consistent behavior

@parthbhatt268
Copy link
Author

parthbhatt268 commented Apr 8, 2025

Hi @Blakko, I addressed your feedback in the core package
Just a quick question: Should we add support in collapse-expand-combo behavior in the pc package? Currently, we use default params if none are passed. If you think we should pass params explicitly from pc as well, can you share how to test this portion.
Thanks

@Blakko
Copy link

Blakko commented Apr 8, 2025

Yeah I think we should if we want this to be accepted upstream.
Best way to test this would be to set up a local demo app and use yarn link to use the development version

@parthbhatt268
Copy link
Author

parthbhatt268 commented Apr 8, 2025

Okay, I did some searching in the repo, but no luck. Could you please share any docs or guidance for setting up the local demo app you mentioned? I am a bit confused here.

I might be missing something here, but I was randomly checking both the repos, and I noticed that in sirensolutions/G6 we have pc but we don't have it in the actual Antv/G6 repo, Is it because it is inside the bundle?

Thanks
image

@Blakko
Copy link

Blakko commented Apr 10, 2025

The app I had is not working anymore due to GAR changes, so we need to set up a new app or update the existing one https://github.com/sirensolutions/g6-sandbox

Upstream you do not see the PC package because you're on the wrong branch. Switch to master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants