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

Replace static Cytoscape plugin with npm package #19127

Open
wants to merge 6 commits into
base: dev
Choose a base branch
from

Conversation

itisAliRH
Copy link
Member

This PR replaces the cytospace visualisation static files with the npm package (@itisalirh/galaxy-cytoscape) developed using @guerler/galaxy-charts-starter. Also removed duplicate elements for "Depth First Search" in the XML file.

How to test the changes?

(Select all options that apply)

  • I've included appropriate automated tests.
  • This is a refactoring of components with existing test coverage.
  • Instructions for manual testing are as follows:
    1. [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]

License

  • I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

@guerler
Copy link
Contributor

guerler commented Nov 11, 2024

Thank you for working on this. Can you set defaults for: search_algorithm, directed and graph_traversal? Also Undirected option has no value. These issues seem to have existed before but we can fix them now I think.

@guerler guerler self-requested a review November 11, 2024 14:56
@itisAliRH
Copy link
Member Author

Thank you for working on this. Can you set defaults for: search_algorithm, directed and graph_traversal? Also Undirected option has no value. These issues seem to have existed before but we can fix them now I think.

The directed option is only used in the styleGenerator and is a boolean. I change it to a boolean input with true/false values.

The search_algorithm and graph_traversal are optional and can be used when the user selects one of them, so the default value for them can be any false values. Should I add <value></value> to the XML file as the default value for those?

@guerler
Copy link
Contributor

guerler commented Nov 11, 2024

Yes, I think we should add explicit values like unset or null or default, so users can switch back to them if necessary. Also let's not use latest as npm package version but an explicit version instead for reproducibility.

@mvdbeek
Copy link
Member

mvdbeek commented Nov 12, 2024

We should publish this under the galaxyproject namespace and not individual namespaces

@mvdbeek mvdbeek requested a review from dannon November 12, 2024 15:13
@guerler
Copy link
Contributor

guerler commented Nov 12, 2024

We should publish this under the galaxyproject namespace and not individual namespaces

Why?

itisAliRH and others added 4 commits November 13, 2024 10:48
Refined the npm package version for Cytoscape to 3.30.3-2
Removed unnecessary options and streamlined the edge type selection to a boolean format.
Copy link
Contributor

@guerler guerler left a comment

Choose a reason for hiding this comment

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

Package has been moved to @galaxyproject as per request of @mvdbeek. Thank you @itisAliRH.

@itisAliRH
Copy link
Member Author

@guerler Thank you for your review and help.

Failure tests are irrelevant and will be fixed in #19138

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

Successfully merging this pull request may close these issues.

3 participants