-
Notifications
You must be signed in to change notification settings - Fork 178
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: Remove direction from NavigationOptions
and drop constructor
#2345
refactor: Remove direction from NavigationOptions
and drop constructor
#2345
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2345 +/- ##
==========================================
- Coverage 49.63% 49.63% -0.01%
==========================================
Files 453 453
Lines 25540 25530 -10
Branches 11709 11705 -4
==========================================
- Hits 12678 12672 -6
+ Misses 4580 4579 -1
+ Partials 8282 8279 -3
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
📊: Physics performance monitoring for 08a3db7physmon summary
|
apparently this is fixing a small navigation issue. when we target a layer in backward navigation we used to propagate one step in the wrong direction and proceed to the target. this is not the case anymore and therefore results are slightly changing |
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.
Let's get it in!
I propose to drop the navigation direction from
NavigationOptions
as the direction vector parameter in the lookup functions is sufficient to provide the same functionality.Additionally I removed the constructor of
NavigationOptions
which duplicated default values and was less explicit. I also removed the magic overstepping limit which will be overwritten by the one from the stepper anyways.