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

Eliminate/Restrict Jquery use for cornerstone-tools #215

Closed
louisgv opened this Issue Nov 8, 2017 · 7 comments

Comments

Projects
None yet
7 participants
@louisgv
Copy link
Contributor

louisgv commented Nov 8, 2017

Expected Behavior

Related to #197 per @vessemer request.

The front-end should not be depending on JQuery as a dependence, unless there is popular demand from contributor. Per #197, it is being imported due to another dependency for advanced medical image processing called cornerstone-tool.

It is desirable if we can devise a minimal version of JQuery that has all the required function that CornerStone-tool need, or simply find a way to remove JQuery.

Current Behavior

The front-end application starting after #197 will be relying on JQuery as a dependency.

Possible Solution

  • Searching for external.$ in cornerstone-tool repository, then log down all method the library is relying on.
  • Extract those method from jQuery and export them into an internal library, with which we expose it to cornerstoneTools
  • Potentially make PR upstream to cornerstone-tool

Context (Environment)

  • JQuery is a sizable library, however we're already using vue.js and since most of the feature we need from JQuery can easily be implemented with vue, it is necessary to not be depended on jQuery.

Detailed Description

Possible Implementation

Checklist before submitting

  • [ + ] I have confirmed this using the officially supported Docker Compose setup using the local.yml configuration and ensured that I built the containers again and they reflect the most recent version of the project at the HEAD commit on the master branch
  • [ + ] I have searched through the other currently open issues and am confident this is not a duplicate of an existing bug
  • [ + ] I provided a minimal code snippet or list of steps that reproduces the bug.
  • I provided screenshots where appropriate
  • [ + ] I filled out all the relevant sections of this template
@truefalse10

This comment has been minimized.

Copy link
Contributor

truefalse10 commented Nov 14, 2017

is anyone working on this right now? otherwise i would start

@vessemer vessemer referenced this issue Nov 15, 2017

Merged

#205 Export pdf functionality. #225

1 of 1 task complete
@Serhiy-Shekhovtsov

This comment has been minimized.

Copy link
Contributor

Serhiy-Shekhovtsov commented Nov 15, 2017

@truefalse10 btw, looks like they have a branch for this: https://github.com/chafey/cornerstone/tree/zachasme-es6
More details here and here

@dannyrb

This comment has been minimized.

Copy link

dannyrb commented Nov 15, 2017

It's worth noting that cornerstone is on track to drop its dependency on jQuery. Work is actively being done. It might be worth waiting for the official change, and then update to latest.

@swederik

This comment has been minimized.

Copy link

swederik commented Dec 13, 2017

We just released Cornerstone Tools 2.0.0 which no longer depends on jQuery. Let us know if you run into any bugs. Thanks!

@lamby

This comment has been minimized.

Copy link
Contributor

lamby commented Dec 28, 2017

ca6f654 :)

@dannyrb

This comment has been minimized.

Copy link

dannyrb commented Dec 28, 2017

@lamby, if you have other Cornerstone related requests, please feel free to cross-post issues. Always happy to hear about others using Cornerstone, and we want to do what we can to make it more useful for integrators

@Serhiy-Shekhovtsov

This comment has been minimized.

Copy link
Contributor

Serhiy-Shekhovtsov commented Jan 4, 2018

This can be closed. Implemented in #269.

@reubano reubano closed this Jan 4, 2018

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