-
Notifications
You must be signed in to change notification settings - Fork 0
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
SAA-459: Remove use of Jquery code, and upgrade peer dependency of Jquery from moj-frontend #167
Conversation
smcveigh941
commented
Mar 12, 2023
•
edited
Loading
edited
- Remove jQuery-UI
- Remove jQuery code. jQuery still required as peer dependency of moj-frontend
- Bumped dependency versions
var valueA = this.getCellValue(tdA) | ||
var valueB = this.getCellValue(tdB) | ||
var valueA = this.getCellValue($(tdA)) | ||
var valueB = this.getCellValue($(tdB)) |
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.
The function from moj-library requires this parameter to be a Jquery object. sigh
@@ -113,16 +113,15 @@ | |||
"govuk-frontend": "^4.5.0", | |||
"helmet": "^6.0.0", | |||
"http-errors": "^2.0.0", | |||
"jquery": "^3.6.3", | |||
"jquery-ui-dist": "^1.13.2", | |||
"jquery": "^3.6.4", |
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.
jQuery library to be removed in the future. However is still required for now since the MoJ library components require it. Considering raising with frontend team about removing the need for jQuery, though it would require a major version change of that library.
Bumping the version for now to suppress vulnerabilities.
Note issue: ministryofjustice/moj-frontend#134
77a4bd1
to
cbcb98c
Compare
…uery from moj-frontend
cbcb98c
to
3d87eaa
Compare
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.
LGTM
@@ -4,17 +4,17 @@ ActivitiesFrontend.ListFilter = function (container) { | |||
new MOJFrontend.FilterToggleButton({ | |||
startHidden: true, | |||
toggleButton: { | |||
container: $('.moj-action-bar__filter'), | |||
container: document.getElementsByClassName('.moj-action-bar__filter'), |
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.
Need to remove the dot at the start of the class name, or you could use query selector,
document.querySelectorAll('.moj-action-bar__filter')