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-6586 Manual start for Navigator journey #5242

Merged
merged 4 commits into from
Jan 30, 2025
Merged

DT-6586 Manual start for Navigator journey #5242

merged 4 commits into from
Jan 30, 2025

Conversation

partisaani
Copy link
Contributor

Proposed Changes

  • Refactored <button> element from NaviCardContainer into NaviCard as the element is tightly coupled with the card.
  • Adds a component which houses a functionality for user to initialize their Navigator journey ahead of its schedule with the following logic: if the first leg is...
  1. a walking leg, the walk leg is set to begin immediately, and its start and end times are shifted. A gap appears between the first and second legs and thus a wait is added between them.
  2. a transit leg, a forceStart flag is added to the transit leg object which is then used as a signal to instruct the user to wait for their transit

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

@partisaani partisaani self-assigned this Jan 28, 2025
return (
<button
type="button"
className={`navi-top-card ${cardExpanded ? 'expanded' : ''}`}
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be nicer to use the classnames library for this (and other similar strings).
Like this:
className={cx('navi-top-card', {expanded: cardExpanded})}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So far I've used cx for cases where there's two or classes which are assigned based on condition. In single cases such as this I find it doesn't really clarify things, and the function call overhead cannot be easily justified.

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.

I noticed that when itinerary has a walk leg at start and you start the trip early, walk instructions appear immediately, then slide up and return again. This is undesirable. Is that something top be fixed in a new PR or should it be fixed within this PR?

@partisaani
Copy link
Contributor Author

partisaani commented Jan 29, 2025

I noticed that when itinerary has a walk leg at start and you start the trip early, walk instructions appear immediately, then slide up and return again. This is undesirable. Is that something to be fixed in a new PR or should it be fixed within this PR?

This issue also occurs whenever cards are swapped during a trip, so I feel like it may deserve a ticket/fix of its own.

@vesameskanen vesameskanen merged commit 8dc44ca into v3 Jan 30, 2025
6 checks passed
@vesameskanen vesameskanen deleted the DT-6586 branch January 30, 2025 07:37
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