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

refactor: use URLSearchParams to manage params #38

Merged
merged 1 commit into from
Aug 16, 2023

Conversation

cdeery
Copy link
Contributor

@cdeery cdeery commented Aug 15, 2023

Instead of managing URL parameters by hand, use the URLSearchParams feature.
This will help with future work managing additional parameters in the future.

@jsnwesson
Copy link
Member

LGTM! My one nitpick is that the URL param itself is 2U_degrees (plural) but its counterpart is singular. If there's at least a note to explain the difference, that'd be great, but I also wonder why we don't just convert the param to be singular as well

@cdeery
Copy link
Contributor Author

cdeery commented Aug 16, 2023

LGTM! My one nitpick is that the URL param itself is 2U_degrees (plural) but its counterpart is singular. If there's at least a note to explain the difference, that'd be great, but I also wonder why we don't just convert the param to be singular as well

Thanks for noticing. The difference between 2u_degree and 2u_degrees is a temporary change to "disable" the degrees LOB without having to modify prospectus. My preference would be to eventually simplify all of the query parameters, because they shouldn't have to be the same as the Algolia parameters.

@cdeery cdeery merged commit 75ae84a into main Aug 16, 2023
4 checks passed
@cdeery cdeery deleted the cdeery/APER-2777/URLSearchParams branch August 16, 2023 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants