removed the function for getURLParam and replaced with URLSearchParam method #9768
Conversation
Manage this branch in SquashTest this branch here: https://lovelyfin00replace-geturlparam-0c4lw.squash.io |
There was a problem hiding this comment.
Great start on this - thanks for getting a PR up.
The code doesn't look right as is - I have put a few comments.
Reach out on Slack if this doesn't make sense.
Also - when changing code - be sure you validate the behaviour before and after. Maybe add console.log to the search function when working locally to understand what this code is doing and when it's called.
My concern is that this change as is may not work correctly.
client/src/entrypoints/admin/core.js
Outdated
| return ''; | ||
| }; | ||
| const searchParams = new URLSearchParams(window.location.search); | ||
| searchParams.set('q', newQuery); |
There was a problem hiding this comment.
I think there may be some confusion here.
You should probably move the new URL... Thing up to line 270 - inside the function body of search
Also I do not see a need to set the query here at all - that's only needed if you intend to update the URL
client/src/entrypoints/admin/core.js
Outdated
| @@ -264,12 +264,12 @@ $(() => { | |||
|
|
|||
| // auto focus on search box | |||
| $input.trigger('focus'); | |||
| const newQuery = $input.val(); | |||
There was a problem hiding this comment.
I don't think this should have moved, pulling this out of the search function will likely break the header search.
This code reads the search field value, which I assume we would only want to do when we call the search function.
lb-
added
component:Search
status:Needs Work
javascript
|
I've made the changes as requested.
|
lb-
force-pushed
the
replace-getURLParam
branch
from
c64338b
to
6d4e785
Compare
- Fixes wagtail#9765 - removed the function for getURLParam function and replaced with URLSearchParam method - already used in many other places in the same file
lb-
force-pushed
the
replace-getURLParam
branch
from
6d4e785
to
426b6da
Compare
Regression from wagtail#9768 Fixes wagtail#9918
Fixes #9765
Fix Summary
npm run lintline 307 gave me errors which I assume is because of this eslint rule// eslint-disable-next-line func-namesnewQuerylocal variable in line 270 was used insearchParams.set(), I had to remove it from that line and move it to line 267.