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: Component hierarchy and code cleanup #1319

Merged
merged 1 commit into from
Feb 21, 2025

Conversation

alexhancock
Copy link
Collaborator

@alexhancock alexhancock commented Feb 20, 2025

A major refactor of component hierarchy and code cleanup

  • No visual or functional changes
  • Calls everything that can render as a full screen experience a "View"
  • Sets up a clear hierarchy of how these views are rendered by using
    • renderer.tsx > App.tsx -> ___View.tsx
    • renderer.tsx only includes top level context providers/error boundary/etc
    • App.tsx is now the top level component which manages the view/setView state
    • Each screen in the app is now in a single component called SomethingView.tsx and these are rendered conditionally based on the value of view in App.tsx
  • Removed a lot of dead / unused / non-working code
  • Eliminates several directories which only had one or two files in them to have a simpler set of top level source code dirs

@nahiyankhan
Copy link
Collaborator

this is huge! digging in.

@alexhancock alexhancock force-pushed the alexhancock/component-hierarchy-cleanup branch from 875834f to c652123 Compare February 20, 2025 22:26
@@ -0,0 +1,9 @@
# Goosey
Copy link
Collaborator

Choose a reason for hiding this comment

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

wait how does this work?

@@ -101,18 +177,42 @@ export default function App() {
isSubmitting={isInstalling}
/>
)}
<ModelProvider>
<ActiveKeysProvider>
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think removing ActiveKeysProvider and ModelProvider will break how we track configured providers and currently set model

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 got moved somewhere and im just not seeing it

<ErrorBoundary>
<App />
</ErrorBoundary>
<ModelProvider>
Copy link
Collaborator

Choose a reason for hiding this comment

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

nvm re earlier comments i found it! nice

@alexhancock alexhancock force-pushed the alexhancock/component-hierarchy-cleanup branch from c652123 to 0f3066c Compare February 21, 2025 15:37
@alexhancock alexhancock force-pushed the alexhancock/component-hierarchy-cleanup branch from 0f3066c to 520fad0 Compare February 21, 2025 16:25
@alexhancock alexhancock merged commit 81a0334 into main Feb 21, 2025
6 checks passed
@alexhancock alexhancock deleted the alexhancock/component-hierarchy-cleanup branch February 21, 2025 16:29
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