feat(ui): Introduce useUI Hook and UIContext#5488
feat(ui): Introduce useUI Hook and UIContext#5488jacob314 merged 1 commit intogoogle-gemini:mainfrom
Conversation
There was a problem hiding this comment.
Summary of Changes
Hello @Lyonk71, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
I've introduced the foundational UIContext and useUI hook. This is a crucial first step towards decoupling UI state management from its presentation, providing a centralized context for UI-related actions across the component tree. This change addresses issue #5446.
Highlights
- New UI Context and Hook: I've added packages/cli/src/ui/hooks/useUI.ts, which defines the UIContext using React's createContext and the useUI hook. This hook provides access to a set of UIContextActions such as openHelp, openAuthDialog, toggleCorgiMode, and quit, enabling centralized UI state management.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces the foundational UIContext and useUI hook to start decoupling UI state management from presentation components. The implementation is solid, following standard React patterns for context creation and consumption. The useUI hook correctly ensures it's used within a provider wrapper, which is a good practice. This change sets a good foundation for the planned refactoring and I have no concerns with the code as written.
jacob314
left a comment
There was a problem hiding this comment.
This looks reasonable. Can you include more of the functionality using in the pr to help judge how it works out in practice_
a63ded7 to
2bc6359
Compare
2bc6359 to
23f4413
Compare
There was a problem hiding this comment.
if we stick with having this context as a follow up it would be nice to structure this more. it has core chat history data, a bunch of things that are generally just plumbed through from settings, some history stuff, a bunch of prop-drill input state that would be good to group together.
There was a problem hiding this comment.
Agreed. Definitely intended to be a part 2 for this refactor - I kept this as simple as possible for now for ease of implementation/merge conflicts.
There was a problem hiding this comment.
UIStateContext worries me a bit. This is a ton of info to be promoting to a context. Is this really needed? Would be better to explicitly pass these around where needed with prop drilling unless there are reasons we really need to do this to achieve this refactor.
There was a problem hiding this comment.
Fair point - I intentionally overused context here as an expediency. It was faster to implement and easier to handle the merge-conflict load. But now that the brunt of the refactor work is done, we'll be able to re-introduce prop usage over time.
There was a problem hiding this comment.
this is a case where we should pass the version in directly rather than using appContext just to get the version. That will keep the app efficient and explicit.
There was a problem hiding this comment.
Good call, change has been implemented.
There was a problem hiding this comment.
this is a case where the uiState generally makes sense. I would suggest we pass it in as a property instead of using Context.
There was a problem hiding this comment.
This is a good callout - I figured this is the sort of thing that could be in a followup refactor, refactor-then-optimize?
There was a problem hiding this comment.
I would suggest making these properties explicit or having a DialogState sub object on uiState.
There was a problem hiding this comment.
Same as previous comment - definitely agree. I lean towards breaking this up in a follow up refactor?
There was a problem hiding this comment.
sounds good. lets land this and then worry about that.
There was a problem hiding this comment.
make this a helper method and separate all the dialog specific state a seperate interface under uiState just for the dialogState.
There was a problem hiding this comment.
Similar to what i mentioned above - this will be a good candidate for a follow up PR to break up UIStateContext
f937287 to
cec6f7c
Compare
There was a problem hiding this comment.
I may have missed it, but where was this logic moved to?
There was a problem hiding this comment.
Spot on, Abhi. Looks like I must've borked a merge conflict somewhere along the way. I'll follow up when I get this resolved.
There was a problem hiding this comment.
I reintroduced the fallback handler into the new architecture + tested it out locally. Thanks for spotting this!
73aede4 to
463cfba
Compare
There was a problem hiding this comment.
nit: are these changes needed?
There was a problem hiding this comment.
Nope, these were left over from local testing. I just removed them.
There was a problem hiding this comment.
why did this output change?
There was a problem hiding this comment.
I didn't intend to push changes to this - I was just doing some local testing. reverted.
36c3eb8 to
96c1fcb
Compare
jacob314
left a comment
There was a problem hiding this comment.
This is a major and impressive architectural refactor. It does an excellent job of separating state management from the UI presentation, which will significantly improve maintainability. The introduction of AppContainer as a central state hub and the decomposition of the old App.tsx into smaller, focused components is a huge step forward.
Here is a summary of my review based on the steps you provided.
I've reviewed the diff with a focus on ensuring no functionality was lost during the refactor.
1. Functionality Preservation:
The refactor appears to be complete and correct. All the state, effects, and UI logic from the monolithic App.tsx have been successfully relocated to the new architecture:
- State and business logic hooks have been moved to
AppContainer.tsx. - State is exposed globally through
UIStateContextandAppContext. - Actions are exposed via
UIActionsContext. - The UI has been cleanly decomposed into new components:
MainContent,DialogManager,Composer,Notifications, andAppHeader.
I did not find any features or logic that were dropped. The implementation seems to have been moved, not removed.
3. Code Quality and Consistency:
- The code quality is very high. The new structure is clean, modular, and aligns with modern React best practices.
- The inclusion of Mermaid diagrams (
/docs/mermaid/) to document the new architecture is an excellent practice and very helpful for understanding the changes.
There was a problem hiding this comment.
is this needed? if so can you switch to the standard wrap with providers from the test helpers? suspect this isn't needed as this change was briefly in main but backed out as I recall.
There was a problem hiding this comment.
Hmm. At least for the moment, yes because the standard renderWithProviders helpers doesn't give the ConfigContext that the component needs for testing. But probably something that can be cleaned up at some point?
There was a problem hiding this comment.
sounds good to cleanup later.
There was a problem hiding this comment.
sounds good. lets land this and then worry about that.
There was a problem hiding this comment.
revert this change. No need to make this async
There was a problem hiding this comment.
revert this one the code is not async.
There was a problem hiding this comment.
this seems like a fine change but unrelated. can you revert it from here and land it separately.
There was a problem hiding this comment.
can we revert "skipLibCheck"?
96c1fcb to
f353fdc
Compare
f353fdc to
04e4047
Compare
| } | ||
|
|
||
| trustedFolders.setValue(cwd, trustLevel); | ||
| const newIsTrusted = |
There was a problem hiding this comment.
Why was this restart logic removed?
There was a problem hiding this comment.
merge conflict. please send a per to add this back.
There was a problem hiding this comment.
Thanks for sharing @shrutip90. Tracking the issue here: #8031 I'll be free to get a PR up to fix this in a couple of hours.
There was a problem hiding this comment.
Thanks for the update, @Lyank71. Confirming that re-introducing the restart logic for folder trust changes is the correct approach. Let me know if you encounter any issues when creating the PR.
There was a problem hiding this comment.
Thanks for the update, @shrutip90! Appreciate you getting a PR up so quickly to address this.
Co-authored-by: Jacob Richman <jacob314@gmail.com>

This commit introduces the foundationalUIContextanduseUIhook.This is the first step in decoupling the UI's state management from its presentation. It creates a centralized context for providing UI-related actions, like opening dialogs, to the entire component tree.This change addresses issue #5446.This PR has been expanded to include the whole of phase 1 described in #5399
This commit introduces a major architectural refactor to the CLI's React UI. It establishes a clear separation between state management and presentational components by centralizing application logic and state into a new
AppContainercomponent.Key Changes
AppContaineras State HubAppContainer.tsxcomponent now wraps theAppcomponent.useHistory,useSlashCommandProcessor,useGeminiStream,useConsoleMessages, etc.) and managing the entire lifecycle of the UI state.Centralized State via React Contexts
UIStateContext: Provides a single, comprehensiveUIStateobject containing all dynamic data for the UI (e.g., history, dialog visibility, streaming status, input buffer).UIActionsContext: Exposes a stable set ofUIActions(e.g.,handleFinalSubmit,handleThemeSelect,handleClearScreen) for components to dispatch events and trigger state changes.AppContext,ConfigContext,SettingsContext: These contexts provide global, relatively static data (version, config, settings) to the component tree, avoiding the need for prop drilling.Presentational
App.tsxApp.tsxcomponent has been significantly simplified. It is now a purely presentational component that consumes state and actions from the new contexts via theuseUIStateanduseUIActionshooks. Its role is to structure the layout of the main UI components.Simplified Hooks and Components
useAuthCommandanduseThemeCommandno longer manage their own dialog visibility state. This state is lifted into the centralUIState, and the hooks now focus purely on the logic of their respective commands.Composer.tsxnow pull data directly from context instead of receiving a large number of props, simplifying their API and improving readability.Pre-React Initialization
initializeAppfunction (packages/cli/src/core/initializer.ts) handles critical startup tasks (auth, theme validation) before the React UI is rendered, ensuring a cleaner separation of concerns.