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

fix(saved-trip-editor): Route back button to saved trips if editing #1255

Merged
merged 5 commits into from
Aug 20, 2024

Conversation

amy-corson-ibigroup
Copy link
Contributor

Description:

Fixes a bug where, if a user accessed the "edit trip" page through a link in their email, the "back" button would send them to a broken Auth0 url.

Now if a user is editing a trip, it will always take you back to saved trips.

PR Checklist:

  • Does the code follow accessibility standards (WCAG 2.1 AA Compliant)?
  • Are all languages supported (Internationalization/Localization)?
  • Are appropriate Typescript types implemented?

Copy link
Collaborator

@binh-dam-ibigroup binh-dam-ibigroup left a comment

Choose a reason for hiding this comment

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

What do you think about using a link to "/" when creating trips?

@@ -12,14 +12,18 @@ const StyledButton = styled(Button)`
padding: 0;
`

export const backButtonContent = (
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe this should be a component.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call! Fixed in 127475c

{/* If creating, back button should return to trip planner. If not, it should return
to saved trips, just in case the user accessed this page through their email. */}
{isCreating ? (
<BackLink />
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we are doing links, how about we place a <Link to="/"> for when isCreating is true? (The link will preserve query params in the URL)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think you're right, added in da8783a

Copy link
Collaborator

@binh-dam-ibigroup binh-dam-ibigroup left a comment

Choose a reason for hiding this comment

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

Sweet!

@amy-corson-ibigroup
Copy link
Contributor Author

@miles-grant-ibigroup @binh-dam-ibigroup if this is looking okay after Miles reviews, and you guys wouldn't mind merging while I'm out next week, that would be amazing!

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.

I'm tempted to rename Content to Text or similar but it's not worth blocking this whole thing

@miles-grant-ibigroup miles-grant-ibigroup merged commit 95b87c9 into dev Aug 20, 2024
9 checks passed
@miles-grant-ibigroup miles-grant-ibigroup deleted the saved-trip-back-link branch August 20, 2024 15:00
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