-
-
Notifications
You must be signed in to change notification settings - Fork 8.8k
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
[JENKINS-64291] Fix drag & drop issues replacing the drag & drop library #5177
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tested this functionally and I can't find any issues, so I vote let's replace the old yui library.
Cleanup needed here first before doing that though
Thanks!
ea52534
to
420bbac
Compare
I'm really excited for any removal of YUI. Lemme know how I can help. |
Co-authored-by: Gavin Mogan <[email protected]>
There's 6 tests failing all seem to be for the same cause: https://github.com/jenkinsci/jenkins/pull/5177/checks?check_run_id=1727548397 |
It may have to do with the removal of the YUI drag & drop module, that it affects the YUI container resizing |
I restored the YUI dragdrop module to see whether it still loads. |
Can you keep drag&drop disabled while resizing enabled? To move forward removing YUI. Not sure whether YUI is structured such a way to allow that though. |
TBH I don't know what the YUI resizing module even does, and I don't know how risky it would be to break it. Given that the main purpose of this PR is a fix for an urgent issue, I'd suggest moving ahead without removing the load of that YUI dragdrop module. We can do that as part of a follow-up ticket. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested manually without issues.
This PR is now ready for merge, after ~24 hours, we will merge it if there's no negative feedback. Thanks! |
See JENKINS-64291.
I haven't found a way to fix JENKINS-64291 other than straight up replacing the drag & drop library. This PR replaces the YUI drag & drop with https://github.com/SortableJS
Where to test:
TODOs
Proposed changelog entries
Proposed upgrade guidelines
N/A
Submitter checklist
Proposed changelog entries
section only if there are breaking changes or other changes which may require extra steps from users during the upgradeDesired reviewers
@timja
@oleg-nenashev
@Wadeck
@halkeye
Maintainer checklist
Before the changes are marked as
ready-for-merge
:Proposed changelog entries
are correctupgrade-guide-needed
label is set and there is aProposed upgrade guidelines
section in the PR title. (example)lts-candidate
to be considered (see query).