-
Notifications
You must be signed in to change notification settings - Fork 641
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
Added null checks to mitigate exceptions on the '/profiles' pages #9388
Conversation
…, so I added checks in the script
submitSearchForm(); | ||
}); | ||
if (typeof searchForm != "undefined" && | ||
typeof searchForm.sortby != "undefined") { // /profiles pages use this js file too, but do not have a searchForm element |
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.
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.
Furthermore you can simplify the code by just checking if searchForm
is falsy, like if (searchForm)
.
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 changed to code to check using just if (searchForm)
now. Thanks!
}); | ||
if (typeof searchForm != "undefined" && | ||
typeof searchForm.sortby != "undefined") { // /profiles pages use this js file too, but do not have a searchForm element | ||
searchForm.sortby.addEventListener('change', (e) => { |
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.
Does that work if we only check once but register them under one block?
Something like:
if (typeof searchForm != "undefined") {
searchForm.sortby.addEventListener('change', (e) => {
searchForm.sortby.value = e.target.value;
submitSearchForm();
});
searchForm.addEventListener('submit', submitSearchForm);
initializeFrameworkAndTfmCheckboxes();
}
Please ignore if it doesn't help.
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.
This makes sense and looks much cleaner, so I've put them all in one block. We only need to check if (searchForm)
for all of them.
The
/profiles
pages use the same js script as the search page, but do not have asearchForm
element on the page, so we were getting exceptions from the javascript on the/profiles
pages when we tried to reference the form elements.I've now added checks in the script so that we do not reference anything that doesn't exist on the page.
I also removed a helper method that was not being used by the search page anymore (
AddSortByOption()
).