feat: refactor the agents admin UI layout#22567
Merged
Conversation
- Fix biome formatting (indentation) across all changed files - Remove unused variable 'displayNameField' in ModelForm.tsx - Remove unused import 'formatProviderLabel' in ModelForm.tsx - Remove unused destructured 'label' in ProviderForm.tsx - Add DeleteDialog confirmation to ProviderForm delete action - Wire up model deletion from ModelForm to ModelsSection's DeleteDialog - Fix ProvidersSection to fall back to list view when provider disappears - Replace template string classNames with cn() utility in ProvidersSection and ModelsSection for consistency
- Add aria-label to provider list items for accessibility - Update button labels: 'Connect' → 'Create provider config', 'Save' → 'Save changes' - Add aria-label='Add model' to dropdown trigger button - Update ModelForm submit button text: 'Add' → 'Add model' - Remove ProviderSpecificModelConfigSchema story (feature was removed with modelConfigSchemas.ts)
DanielleMaywood
requested changes
Mar 3, 2026
site/src/pages/AgentsPage/ChatModelAdminPanel/ModelConfigFields.tsx
Outdated
Show resolved
Hide resolved
site/src/pages/AgentsPage/ChatModelAdminPanel/ProvidersSection.tsx
Outdated
Show resolved
Hide resolved
- ModelConfigFields: inline renderProviderSpecificField (only called once) - ModelForm: remove empty try/catch block, replace div role='button' with actual button elements, use formik getFieldProps for displayName - ModelsSection: simplify ternary to function with if statements, use Tailwind divide-y utility instead of conditional borders, replace div role='button' with button element - ProviderForm: replace useMemo with useState lazy initializer for initialValues, remove useEffect sync pattern (parent should handle re-keying), replace div role='button' with button element - ProvidersSection: remove useEffect and do conditional setState during render, replace div role='button' with button element These changes improve: - Accessibility (semantic HTML buttons with native keyboard support) - Code clarity (simpler conditionals, inlined single-use functions) - React best practices (useState lazy init, no prop-to-state sync) - Reduced LOC: 204 lines → 114 lines (-90)
The button elements from the previous commit made the UI look bad. Reverted to div role='button' but with CORRECT keyboard handling based on useClickable: - Enter key: onKeyDown (fires and repeats when held) - Space key: onKeyUp (fires on release, no repeat) - Both: stopPropagation to prevent bubbling This matches native button behavior and maintains accessibility without using the useClickable hook (which can't be used in conditionals or loops). Updated files: - ModelForm.tsx (3 clickable divs) - ModelsSection.tsx (2 clickable divs in map) - ProviderForm.tsx (1 clickable div) - ProvidersSection.tsx (1 clickable div in map)
DanielleMaywood
approved these changes
Mar 3, 2026
Contributor
DanielleMaywood
left a comment
There was a problem hiding this comment.
LGTM, shame about the buttons but I'll revisit this in the future
One 'Back' button was using onKeyDown for both Enter and Space keys, while all other clickable divs correctly use: - onKeyDown for Enter (fires and repeats) - onKeyUp for Space (fires on release, no repeat) This inconsistency is now fixed for accessibility.
- EnvPresetProviders: navigate list→detail→back→detail per new pattern - CreateAndUpdateProvider: wait for form to transition to update mode after create, then perform update without navigating away - NoModelConfigByDefault, SubmitModelConfigExplicitly, ValidatesModelConfigFields: add openAddModelForm helper that clicks the dropdown trigger then selects provider from the menu - Fix 'Model ID' → 'Model Identifier' label mismatch - Fix 'Reasoning effort' → 'Reasoning Effort' label mismatch
- CreateAndUpdateProvider: clear API key field before retyping - NoModelConfigByDefault, SubmitModelConfigExplicitly, ValidatesModelConfigFields: click 'Advanced' toggle to reveal Max Output Tokens field; use waitFor for dropdown menuitem All 6 ChatModelAdminPanel interaction tests passing locally.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I am working on a subsequent change to make the fields auto-generated with
make genfrom the Go code itself, rather than us needing to create a UI compatibility layer.Once the above is done, I'll be adding in the payload so users can very easily just click "Opus 4.6" to add the model, and the config values will be set appropriately.
This is really just UI changes, nothing functionally should change here. But the code will be cleaned up a lot post the above changes.