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

🪟🔧 Refactor frontend destination routing #19120

Merged
merged 7 commits into from
Nov 16, 2022
Merged

Conversation

josephkmh
Copy link
Contributor

@josephkmh josephkmh commented Nov 8, 2022

What

This PR refactors the routes nested in the destination path: /workspaces/:workspaceId/destination

With the aim of:

  1. Moving components out of pages and into the components directory
  2. Surfacing routes in the routes.tsx file for less nesting and better discoverability
  3. Separating pure routing concerns (<Route> components) from application specific components
  4. Adding consistency to how we define route paths

Part of a larger effort to clean up our frontend routing logic.

How

  • Components that were located next to pages have been moved to src/components/destination
  • All <Route> components have been moved up to routes.tsx so that the whole route tree becomes more visible (see these changes)
  • Nested <Routes> have been replaced with react-router's <Outlet> component

Recommended reading order

  1. routes.tsx and routePaths.tsx to see the broad picture
  2. pages/destination to see the purely routing components
  3. src/components/destination to see the application components related to the destination app domain

Next steps

A similar refactor will be done for sources, connections and settings so that we do not have so many deeply nested and conditional <Route> components.

@github-actions github-actions bot added area/platform issues related to the platform area/frontend Related to the Airbyte webapp labels Nov 8, 2022
Comment on lines +48 to +56
<Route path={RoutePaths.Destination}>
<Route index element={<AllDestinationsPage />} />
<Route path={DestinationPaths.NewDestination} element={<CreateDestinationPage />} />
<Route path={DestinationPaths.NewConnection} element={<CreationFormPage />} />
<Route path={DestinationPaths.Root} element={<DestinationItemPage />}>
<Route path={DestinationPaths.Settings} element={<DestinationSettingsPage />} />
<Route index element={<DestinationOverviewPage />} />
</Route>
</Route>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the main improvement introduced in this PR - we can see the route structure clearly.

@josephkmh josephkmh marked this pull request as ready for review November 15, 2022 13:22
@josephkmh josephkmh requested a review from a team as a code owner November 15, 2022 13:22
@josephkmh josephkmh requested a review from timroes November 15, 2022 13:23
@josephkmh josephkmh changed the title 🪟🔧 [DRAFT] Refactor frontend destination routing 🪟🔧 Refactor frontend destination routing Nov 15, 2022
Copy link
Contributor

@krishnaglick krishnaglick left a comment

Choose a reason for hiding this comment

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

I like the work being done here! Did some light testing, clicking around in OSS and that all seems to work 👍

@josephkmh josephkmh merged commit 7bdefbc into master Nov 16, 2022
@josephkmh josephkmh deleted the joey/refactor-routing branch November 16, 2022 09:17
akashkulk pushed a commit that referenced this pull request Dec 2, 2022
* move components out of pages directory

* flatten destination routes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/frontend Related to the Airbyte webapp area/platform issues related to the platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants