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

Jquery included #417

Merged
merged 2 commits into from
Feb 5, 2021
Merged

Jquery included #417

merged 2 commits into from
Feb 5, 2021

Conversation

will-moore
Copy link
Member

@will-moore will-moore commented Dec 17, 2020

This PR updates OMERO.figure to host jquery.js, jquery-ui.js and jquery-ui.css instead of loading them from omero-web since they are all updated in ome/omero-web#180 and bootstrap 3.0.0 is not compatible with latest jquery. We load the exact same versions of jquery and jquery-ui, so there shouldn't be any issues. However, I haven't included all the jquery-ui.css since most of it shouldn't be needed.

According to twbs/bootstrap#16834 (comment) "Bootstrap 3.3.7 now supports jQuery 3+." but this would require a lot of upgrade work on bootstrap in OMERO.figure, so this is a shorter-term fix to allow omero-web to upgrade jquery.

To test:

  • App should still be working. No JavaScript issues expected - same libraries are loaded.
  • No css issues (e.g. with jQuery UI components such as sliders or tabs)

@will-moore
Copy link
Member Author

This has been in the merge-ci deployment for a while, with no issues noticed.
Now excluding ome/omero-web#180, to verify that OMERO.figure with this PR can be released independently of the omero-web jQuery update....

@jburel
Copy link
Member

jburel commented Jan 26, 2021

is the plan to make figure "independent" of the dependency added by omero-web for now? then loads the dependencies from omero-web after making changes to upgrade bootstrap.
Or include the dependencies in figure even after the upgrade mentioned above.
If any case, this should be documented.

@will-moore
Copy link
Member Author

I think that figure should permanently host its own copy of jquery, just as it does with all other dependencies.
This is really the ending of the "exceptional" behaviour of sharing jquery with omero-web.
This behaviour hasn't been documented before since it's really internal behaviour and doesn't affect users or even developers.
And now that is ending, so I don't think it will be of much interest to document?

@jburel
Copy link
Member

jburel commented Jan 26, 2021

I agree that being "web independent" is preferable.
We should, at least, create an issue indicating that an upgrade of jquery is required for figure since as mentioned in the description this is a shorter-term fix...

@will-moore
Copy link
Member Author

Issue created at #423

@jburel jburel merged commit 7cc77ae into ome:master Feb 5, 2021
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.

2 participants