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

frontend: add custom react-router hooks #1577

Merged
merged 4 commits into from
Jul 7, 2021
Merged

frontend: add custom react-router hooks #1577

merged 4 commits into from
Jul 7, 2021

Conversation

dschaller
Copy link
Contributor

@dschaller dschaller commented Jul 2, 2021

Description

Add custom react-router hooks to:

  • preserve utm search params
  • provide an easier interface to track origins when routing between pages
  • provide an easier interface to specify search params (see this for context)

Testing Performed

manual and unit

@dschaller dschaller requested a review from a team as a code owner July 2, 2021 06:47
frontend/packages/core/src/navigation.tsx Show resolved Hide resolved
} else {
navPath = { pathname: to?.pathname, hash: to?.hash };
}
const [path, search] = navPath.pathname.split("?");
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any utility that would simplify parsing the URL rather than needing to split?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There isn't an easy way to split this since it's just the path. If I wanted to use a utility I would need to rely on the window location to get the hostname.

mikecutalo
mikecutalo previously approved these changes Jul 2, 2021
Copy link
Contributor

@mikecutalo mikecutalo left a comment

Choose a reason for hiding this comment

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

Do we also need to update the scaffolder to use this new package?

@mikecutalo mikecutalo dismissed their stale review July 2, 2021 20:00

Sorry misfired

@dschaller dschaller merged commit 5f1874d into main Jul 7, 2021
@dschaller dschaller deleted the navigation branch July 7, 2021 21:01
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