-
Notifications
You must be signed in to change notification settings - Fork 46
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
front: enhance pathfinding loading state management #10292
base: dev
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## dev #10292 +/- ##
==========================================
- Coverage 81.85% 81.79% -0.06%
==========================================
Files 1075 1057 -18
Lines 107172 106211 -961
Branches 728 728
==========================================
- Hits 87726 86877 -849
+ Misses 19407 19295 -112
Partials 39 39
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
LGTM and tested !
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.
Tested and everything works great, good job ! Just a few comments.
Also before merging, I would rather show the loader animation on the map to @thibautsailly to be sure it's ok for him since he also requested to @Caracol3 the blue loader below the form.
82d01b4
to
0b3faca
Compare
front/src/modules/trainschedule/components/ManageTrainSchedule/NewMap.tsx
Outdated
Show resolved
Hide resolved
0b3faca
to
228a8c8
Compare
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.
Lgtm and tested, one last comment. Just waiting for you to fix conflicts to test one more time and we should be good to go :)
front/src/modules/trainschedule/components/ManageTrainSchedule/Map.tsx
Outdated
Show resolved
Hide resolved
7349719
to
8cc275a
Compare
This PR can be rebase with last changes from this one :) |
8cc275a
to
1e7311c
Compare
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.
After the rebase, I have 2 behaviors issues. Both are probably related and coming from the switch from useLazyQuery
to useQuery
.
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.
When a pathfinding is loading, it seems the loading banner is always green and not blue
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.
For a path like Brest > Narbonne > Perpignan (but should work for any path with a via), if I remove the origin, pathProperties
endpoint is fired again (but it shouldn't, it's not the case right now) and bbox on map is updated with a path containing the via as if it was the origin and the destination.
When checking the network tab in dev tools, we see only pathProperties
endpoint launching but in its payload, we can see that the track_section_ranges
doesn't have the same length. So rtk query is probably relaunching pathfinding blocks
as well in the dark.
Completes this PR
This PR will add a loader while the pathfinding is still computing, and the map's viewport will be updated only once it is finished, avoiding the 'shaking' effect.