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

Update first-party extensions with changes to 3.0 API #1718

Closed
maxkfranz opened this Issue Feb 21, 2017 · 6 comments

Comments

Projects
None yet
2 participants
@maxkfranz
Member

maxkfranz commented Feb 21, 2017

There are breaking API changes in 3.0.

Test each of the first-party extensions to see if they work already (by swapping their reference of cytoscape.js to a local build of 3.0. If not, revise the calls to use the new API.

Layouts:

  • arbor
  • cola
  • cose-bilkent
  • dagre
  • spread
  • springy

UIs:

  • automove
  • cxtmenu
  • edgehandles
  • navigator
  • panzoom
  • qtip

@maxkfranz maxkfranz added this to the 3.0.0 milestone Feb 21, 2017

@mj3cheun mj3cheun self-assigned this Feb 23, 2017

@mj3cheun

This comment has been minimized.

Show comment
Hide comment
@mj3cheun

mj3cheun Feb 23, 2017

Member

Do you want me to use a copy of cytoscape.js from the 3.x branch or will the unstable branch do?

Member

mj3cheun commented Feb 23, 2017

Do you want me to use a copy of cytoscape.js from the 3.x branch or will the unstable branch do?

@maxkfranz

This comment has been minimized.

Show comment
Hide comment
@maxkfranz

maxkfranz Feb 23, 2017

Member

You can't use unstable, as it doesn't have any 3.0 breaking API changes. The 3.x branch needs to be merged onto master and unstable when 3.0 is released.

Extensions that already work OK on 3.0 can be released as a new minor release with the dependency updated to something like cytoscape: '^2.4 | ^3.0'. Small changes that can easily support both versions can do the same (e.g. event.target || event.cyTarget).

Where the extension could work on 3.0 only by breaking support for 2.x, we'll need to make the changes in a cytoscape3 branch. The branch can be merged in for each of those extensions after 3.0 is officially released.

Probably most extensions will need little or no changes to work with 3.0

Member

maxkfranz commented Feb 23, 2017

You can't use unstable, as it doesn't have any 3.0 breaking API changes. The 3.x branch needs to be merged onto master and unstable when 3.0 is released.

Extensions that already work OK on 3.0 can be released as a new minor release with the dependency updated to something like cytoscape: '^2.4 | ^3.0'. Small changes that can easily support both versions can do the same (e.g. event.target || event.cyTarget).

Where the extension could work on 3.0 only by breaking support for 2.x, we'll need to make the changes in a cytoscape3 branch. The branch can be merged in for each of those extensions after 3.0 is officially released.

Probably most extensions will need little or no changes to work with 3.0

@maxkfranz

This comment has been minimized.

Show comment
Hide comment
@maxkfranz

maxkfranz Feb 23, 2017

Member

@mj3cheun We also should document the cytoscape.use( require('some-extension') ) style in the extensions that are updated to work with 3.0

Member

maxkfranz commented Feb 23, 2017

@mj3cheun We also should document the cytoscape.use( require('some-extension') ) style in the extensions that are updated to work with 3.0

@mj3cheun

This comment has been minimized.

Show comment
Hide comment
@mj3cheun

mj3cheun Feb 24, 2017

Member

@maxkfranz I have updated arbor and cola, you may want to have a look to see if I am doing it correctly. If I am, I will continue to update the rest of the dependencies.

Member

mj3cheun commented Feb 24, 2017

@maxkfranz I have updated arbor and cola, you may want to have a look to see if I am doing it correctly. If I am, I will continue to update the rest of the dependencies.

@maxkfranz

This comment has been minimized.

Show comment
Hide comment
@maxkfranz

maxkfranz Feb 24, 2017

Member

@mj3cheun OK, added some comments. Looks pretty good so far

Member

maxkfranz commented Feb 24, 2017

@mj3cheun OK, added some comments. Looks pretty good so far

@mj3cheun

This comment has been minimized.

Show comment
Hide comment
@mj3cheun

mj3cheun Mar 1, 2017

Member

@maxkfranz Also found that for 3.x after calling the layout function, you need to add a .run() in order to make the layout function actually work. In 2.x, this appears to be done for you so .run() is not required. Is this intentional? See cytoscape/cytoscape.js-automove@b7972e0 for example. This is a breaking change and applies to all the built-in layouts (eg. circle etc.).

Otherwise I am done with all the extensions, which are all backwards compatible.

Member

mj3cheun commented Mar 1, 2017

@maxkfranz Also found that for 3.x after calling the layout function, you need to add a .run() in order to make the layout function actually work. In 2.x, this appears to be done for you so .run() is not required. Is this intentional? See cytoscape/cytoscape.js-automove@b7972e0 for example. This is a breaking change and applies to all the built-in layouts (eg. circle etc.).

Otherwise I am done with all the extensions, which are all backwards compatible.

@maxkfranz maxkfranz closed this Apr 11, 2017

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