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

Migrate frontend navigation to App Navigation Abstraction #8631

Merged
merged 4 commits into from May 13, 2024

Conversation

kmcgrady
Copy link
Collaborator

@kmcgrady kmcgrady commented May 8, 2024

Describe your changes

Combine all logic of the navigation (page url updates, mpa management), and put it in a class (V1AppNavigation). We create a layer above it that essentially pulls from the V1AppNavigation as an overengineering effort to set up for AppNavigation V2

Testing Plan

  • Original tests should pass
  • unit tests for AppNavigation.

Contribution License Agreement

By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.

@kmcgrady kmcgrady force-pushed the refactor/app-navigation-abstraction branch from 15d7faa to 0fcb168 Compare May 10, 2024 01:59
@kmcgrady kmcgrady marked this pull request as ready for review May 10, 2024 20:08
const maybeState = appNavigation.handleNewSession(generateNewSession())
expect(maybeState).not.toBeUndefined()

// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: this shows up enough that I think we might as well disable the eslint rule for the whole file given it's a test file anyway

@kmcgrady kmcgrady merged commit 2d10e6a into feature/mpa-v2 May 13, 2024
31 checks passed
@kmcgrady kmcgrady deleted the refactor/app-navigation-abstraction branch May 13, 2024 17:31
kmcgrady added a commit that referenced this pull request May 20, 2024
## Describe your changes

Combine all logic of the navigation (page url updates, mpa management),
and put it in a class (V1AppNavigation). We create a layer above it that
essentially pulls from the V1AppNavigation as an overengineering effort
to set up for AppNavigation V2

## Testing Plan

- Original tests should pass
- unit tests for AppNavigation.

---

**Contribution License Agreement**

By submitting this pull request you agree that all contributions to this
project are made under the Apache 2.0 license.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants