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

Remove jQuery from 'New item' page #10208

Open
wants to merge 16 commits into
base: master
Choose a base branch
from

Conversation

janfaracik
Copy link
Contributor

@janfaracik janfaracik commented Jan 24, 2025

This PR removes jQuery from the 'New item' page. Removing jQuery simplifies the codebase by relying on modern JavaScript features, which are now supported across all major browsers without the need for jQuery. It's also a reduction in dependencies/JS size, which will make for a faster experience for users.

Testing done

  • New item page works as expected
  • Blank check works
  • Existing name check works
  • Illegal name works
  • Selecting a type of item and then duplicating a project removes the selection
  • Duplicating a project then selecting a type of item removes the duplicated project text

Proposed changelog entries

  • N/A

Proposed upgrade guidelines

N/A

Submitter checklist

Preview Give feedback

Desired reviewers

@jenkinsci/sig-ux

Before the changes are marked as ready-for-merge:

Maintainer checklist

Preview Give feedback

var itemName = document.querySelector(
'#createItem input[name="name"]',
).value;
return itemName.trim() === "";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now prevents names that are just spaces from being submitted.

btn.prop("disabled", true);
}
}
function refreshSubmitButtonState() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rather than accept a true/false parameter lets just have the function handle whether the button should be valid or not.

@krisstern
Copy link
Member

It looks like some JUnit tests are now failing after the removal of JQuery from the "New item" page:

00:47:53  [ERROR] Errors: 
00:47:53  [ERROR]   MyViewTest.testDoCreateItem:93 » Script TypeError: Cannot set property "value" of null to "" (http://localhost:34599/jenkins/static/44ddddd0/jsbundles/add-item.js#96)
00:47:53  [INFO] 
00:47:53  [ERROR] Tests run: 2214, Failures: 0, Errors: 1, Skipped: 55
00:47:53  [INFO] 
00:47:53  [ERROR] 
00:47:53  
00:47:53  See /home/jenkins/agent/workspace/Core_jenkins_PR-10208/test/target/surefire-reports for the individual test results.
00:51:21  [ERROR] Errors: 
00:51:21  [ERROR]   MyViewTest.testDoCreateItem:93 » Script TypeError: Cannot set property "value" of null to "" (http://localhost:38409/jenkins/static/86cd0c78/jsbundles/add-item.js#96)
00:51:21  [INFO] 
00:51:21  [ERROR] Tests run: 2214, Failures: 0, Errors: 1, Skipped: 55
00:51:21  [INFO] 
00:51:21  [ERROR] 
00:51:21  
00:51:21  See /home/jenkins/agent/workspace/Core_jenkins_PR-10208/test/target/surefire-reports for the individual test 

Ref: https://ci.jenkins.io/job/Core/job/jenkins/job/PR-10208/2/console

@janfaracik janfaracik added skip-changelog Should not be shown in the changelog web-ui The PR includes WebUI changes which may need special expertise labels Jan 25, 2025
@janfaracik janfaracik requested a review from a team January 25, 2025 11:17
@janfaracik
Copy link
Contributor Author

It looks like some JUnit tests are now failing after the removal of JQuery from the "New item" page:

00:47:53  [ERROR] Errors: 
00:47:53  [ERROR]   MyViewTest.testDoCreateItem:93 » Script TypeError: Cannot set property "value" of null to "" (http://localhost:34599/jenkins/static/44ddddd0/jsbundles/add-item.js#96)
00:47:53  [INFO] 
00:47:53  [ERROR] Tests run: 2214, Failures: 0, Errors: 1, Skipped: 55
00:47:53  [INFO] 
00:47:53  [ERROR] 
00:47:53  
00:47:53  See /home/jenkins/agent/workspace/Core_jenkins_PR-10208/test/target/surefire-reports for the individual test results.
00:51:21  [ERROR] Errors: 
00:51:21  [ERROR]   MyViewTest.testDoCreateItem:93 » Script TypeError: Cannot set property "value" of null to "" (http://localhost:38409/jenkins/static/86cd0c78/jsbundles/add-item.js#96)
00:51:21  [INFO] 
00:51:21  [ERROR] Tests run: 2214, Failures: 0, Errors: 1, Skipped: 55
00:51:21  [INFO] 
00:51:21  [ERROR] 
00:51:21  
00:51:21  See /home/jenkins/agent/workspace/Core_jenkins_PR-10208/test/target/surefire-reports for the individual test 

Ref: https://ci.jenkins.io/job/Core/job/jenkins/job/PR-10208/2/console

Thanks! Fixed now, forgot to try it with no existing jobs.

@krisstern
Copy link
Member

Thanks! Fixed now, forgot to try it with no existing jobs.

Awesome!

@krisstern krisstern requested a review from a team January 25, 2025 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip-changelog Should not be shown in the changelog web-ui The PR includes WebUI changes which may need special expertise
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants