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 old URL params [OTP-1233] #1226

Merged
merged 9 commits into from
Jun 11, 2024
Merged

Conversation

binh-dam-ibigroup
Copy link
Collaborator

@binh-dam-ibigroup binh-dam-ibigroup commented May 31, 2024

Description

This PR remove some (not all) params that are no longer used in OTP searches from the URL.

PR Checklist

  • Does the code follow accessibility standards (WCAG 2.1 AA Compliant)?
  • Are all languages supported (Internationalization/Localization)?
  • Are appropriate Typescript types implemented?

@binh-dam-ibigroup binh-dam-ibigroup changed the title Remove old URL params Remove old URL params [OTP-1233] Jun 3, 2024
Copy link
Contributor

@amy-corson-ibigroup amy-corson-ibigroup left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks for the changes

Copy link
Contributor

@daniel-heppner-ibigroup daniel-heppner-ibigroup left a comment

Choose a reason for hiding this comment

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

This looks fine, definitely a nice simple solution. I'd like to remove the getDefaultQuery() call entirely though eventually. What are we still using it for? Is it the date/time picker and from/to fields only?

lib/util/api.ts Outdated

/**
* Drops unused params so they don't show up in URL.
* TODO: Remove those params from core-utils.
Copy link
Contributor

Choose a reason for hiding this comment

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

Removing these from core-utils would be a big project, because they're still relied on by some other packages which are used in custom uis by some users. I think we should try to remove the dependency on them from OTP-RR instead.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You are right. The stuff that comes out of getDefaultQuery is not very useful anyway:

bikeSpeed: 3.58
date: "2024-06-11"
departArrive: "NOW"
endTime: "09:00"
from: null
intermediatePlaces: Array []
maxBikeTime: 20
maxEScooterDistance: 4828
maxWalkTime: 15
mode: "WALK,TRANSIT"
numItineraries: 3
otp2: true
routingType: "ITINERARY"
startTime: "07:00"
time: "10:36"
to: null
watts: 250
wheelchair: false

lib/util/api.ts Outdated
* Drops unused params so they don't show up in URL.
* TODO: Remove those params from core-utils.
*/
export function removeUnusedQueryParams(params: any): void {
Copy link
Contributor

Choose a reason for hiding this comment

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

make the type Record<string, any>

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated in b3f93e4

@binh-dam-ibigroup binh-dam-ibigroup merged commit 3031c96 into dev Jun 11, 2024
9 checks passed
@binh-dam-ibigroup binh-dam-ibigroup deleted the remove-old-url-params branch June 11, 2024 14:58
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.

3 participants