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

feat(core-utils): restore walkSpeed param for walk-only trips #813

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

themapwizard
Copy link

@themapwizard themapwizard commented Mar 3, 2025

See PRJ77-715

This PR adds walkSpeed to core-utils\src\query-params.js and adds them to the default params in core-utils\src\query.js.

This is for trimet.org and will add back walkSpeed for walk only modes. We needed to add back walkSpeed because latest versions 12.0.2 do not contain walkSpeed and upgrading in trimet.org would render no settings in the walk only panel (see screenshots).

walk only:
Screenshot 2025-03-03 115125

Copy link
Collaborator

@miles-grant-ibigroup miles-grant-ibigroup left a comment

Choose a reason for hiding this comment

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

Looking at the failing snapshots, it looks like these 2 variables are being applied in many cases where they perhaps should not be. This PR would send a default 2 walkReluctance along in many cases where it perhaps should not be. If this is desired, then I'd consider this a change that might make more sense within the trimet-specific map form component.

@themapwizard themapwizard force-pushed the PRJ77-715 branch 2 times, most recently from f479aca to 4f53792 Compare March 4, 2025 16:30
@themapwizard themapwizard changed the title feat(core-utils): add walkTolerance param and restore walkSpeed param for walk-only trips feat(core-utils): restore walkSpeed param for walk-only trips Mar 4, 2025
@themapwizard
Copy link
Author

Looking at the failing snapshots, it looks like these 2 variables are being applied in many cases where they perhaps should not be. This PR would send a default 2 walkReluctance along in many cases where it perhaps should not be. If this is desired, then I'd consider this a change that might make more sense within the trimet-specific map form component.

this should no longer be an issue, i've update the PR to only add back walkSpeed

Copy link
Collaborator

@miles-grant-ibigroup miles-grant-ibigroup left a comment

Choose a reason for hiding this comment

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

If you go ahead and update the snapshots then I think this is good to go!

@themapwizard
Copy link
Author

If you go ahead and update the snapshots then I think this is good to go!

updated

@miles-grant-ibigroup
Copy link
Collaborator

It doesn't look like that quite worked! Did you try running yarn unit -u?

@themapwizard
Copy link
Author

It doesn't look like that quite worked! Did you try running yarn unit -u?

I originally ran it with jest directly, i just re-ran with yarn unit -u

@daniel-heppner-ibigroup
Copy link
Contributor

Sorry @themapwizard yarn unit -u is actually outdated, we now have a script for updating the storybook snapshots because they require that the storybook be built to HTML. You can run yarn update-snapshots

@daniel-heppner-ibigroup
Copy link
Contributor

I was able to do it myself but I can't push to your branch so you'll have to do it yourself. Sorry for the runaround.

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