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

DT-6533 Improved navigator and local storage cooperation #5162

Merged
merged 12 commits into from
Nov 14, 2024

Conversation

partisaani
Copy link
Contributor

@partisaani partisaani commented Nov 12, 2024

Proposed Changes

Navigator is now more context aware:

  • If from, to, time, arriveBy and hash parameters are identical to those of the stored journey
    • Traversing to ItineraryPage detail view will automatically trigger the Navigator journey.
  • If from, to, time and arriveBy parameters are identical to those of the stored journey
    • Traversing to ItineraryPage list view does not necessarily purge the stored itinerary from memory.
      • Local storage entry is purged if user navigates to list view from active navigator journey either by using the arrow icon or by browsing back using browser history
    • Traversing to ItineraryPage detail view does not purge the stored itinerary from memory, but a Navigator journey is not triggered
  • If any of the parameters is different
    • Traversing to ItineraryPage list view or detail view will purge the stored Navigator journey from local storage

Pull Request Check List

  • A reasonable set of unit tests is included
  • Console does not show new warnings/errors
  • Changes are documented or they are self explanatory
  • This pull request does not have any merge conflicts
  • All existing tests pass in CI build

Review

  • Read and verify the code changes
  • Test the functionality by running the UI locally with all popular browsers available in your platform
  • Check that the implementation matches the design, when such one is defined in a Jira issue
  • Merge the pull request

Copy link
Member

@vesameskanen vesameskanen left a comment

Choose a reason for hiding this comment

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

Note that when you open itinerary detaul view of a bike and transit itinerary, hash is not itinerary index. It can be found from secondaryHash.

As far as I can see, we need to 'auto relaunch' navigation only in one occasion: page reloading. This is handled in useEffect [].

app/component/itinerary/ItineraryPageUtils.js Outdated Show resolved Hide resolved
app/component/itinerary/StartNavi.js Outdated Show resolved Hide resolved
app/component/itinerary/ItineraryPage.js Outdated Show resolved Hide resolved
app/component/itinerary/ItineraryPage.js Outdated Show resolved Hide resolved
app/component/itinerary/ItineraryPage.js Outdated Show resolved Hide resolved
@partisaani
Copy link
Contributor Author

partisaani commented Nov 13, 2024

As far as I can see, we need to 'auto relaunch' navigation only in one occasion: page reloading. This is handled in useEffect [].

Will refactor with this in mind. For possible future reference the current implementation enables the user to browse between itineraries and their detail views. Browsing to the exact same search result auto-launches Navigator.

Only explicit backtrack from an initiated journey or a change in from, to, time or arriveBy params signal to purge the stored itinerary.

Copy link
Member

@vesameskanen vesameskanen left a comment

Choose a reason for hiding this comment

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

very good

@vesameskanen vesameskanen merged commit 3e764c9 into v3 Nov 14, 2024
6 checks passed
@vesameskanen vesameskanen deleted the DT-6476_patch-1 branch November 14, 2024 05:14
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