fix(ui): improve AIConfigPopup heuristics#743
fix(ui): improve AIConfigPopup heuristics#743madhav758 wants to merge 1 commit intoaccordproject:mainfrom
Conversation
Signed-off-by: Shreyansh Madhav <madhavshreyansh@gmail.com>
✅ Deploy Preview for ap-template-playground ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull request overview
Improves the AI configuration modal UX in the Template Playground by reordering key inputs and adding inline heuristics/feedback to reduce user configuration errors.
Changes:
- Moved the API key input above the model name and added an in-field show/hide API key toggle.
- Added provider-specific model “recognized/unrecognized” feedback based on a
KNOWN_MODELSlist. - Hoisted
KNOWN_MODELSoutside the component to avoid hook dependency lint warnings.
Comments suppressed due to low confidence (1)
src/components/AIConfigPopup.tsx:134
showApiKeystate can persist across modal close/reopen because the component returnsnullwhen closed (state is retained). If a user toggles to show the key, closing and reopening could unexpectedly render the key in plain text. Consider resettingshowApiKeytofalsewhenisOpenbecomes true (or on close/reset).
useEffect(() => {
if (isOpen) {
const savedProvider = localStorage.getItem('aiProvider');
const savedModel = localStorage.getItem('aiModel');
const savedApiKey = localStorage.getItem('aiApiKey');
const savedCustomEndpoint = localStorage.getItem('aiCustomEndpoint');
const savedMaxTokens = localStorage.getItem('aiResMaxTokens');
const savedShowFullPrompt = localStorage.getItem('aiShowFullPrompt') === 'true';
const savedEnableCodeSelection = localStorage.getItem('aiEnableCodeSelectionMenu') !== 'false';
const savedEnableInlineSuggestions = localStorage.getItem('aiEnableInlineSuggestions') !== 'false';
if (savedProvider) setProvider(savedProvider);
if (savedModel) setModel(savedModel);
if (savedApiKey) setApiKey(savedApiKey);
if (savedCustomEndpoint) setCustomEndpoint(savedCustomEndpoint);
if (savedMaxTokens) setMaxTokens(savedMaxTokens);
setShowFullPrompt(savedShowFullPrompt);
setEnableCodeSelectionMenu(savedEnableCodeSelection);
setEnableInlineSuggestions(savedEnableInlineSuggestions);
}
}, [isOpen]);
| <button | ||
| type="button" | ||
| onClick={() => setShowApiKey(!showApiKey)} | ||
| className={`absolute inset-y-0 right-0 pr-3 flex items-center ${theme.helpText} hover:text-gray-700`} |
There was a problem hiding this comment.
The toggle button uses a hard-coded hover:text-gray-700, which will likely reduce contrast in dark mode (gray-700 on a dark background). Consider using a theme-aware hover color or conditional classes based on isDarkMode to maintain readable contrast.
| className={`absolute inset-y-0 right-0 pr-3 flex items-center ${theme.helpText} hover:text-gray-700`} | |
| className={`absolute inset-y-0 right-0 pr-3 flex items-center ${theme.helpText} hover:opacity-80`} |
| @@ -215,39 +300,42 @@ const AIConfigPopup = ({ isOpen, onClose, onSave }: AIConfigPopupProps) => { | |||
| value={model} | |||
| onChange={(e) => setModel(e.target.value)} | |||
| placeholder="Enter model name" | |||
| className={`w-full p-2 border rounded-lg focus:outline-none focus:ring-2 ${theme.input}`} | |||
| className={`w-full p-2 border rounded-lg focus:outline-none focus:ring-2 ${theme.input} ${isKnownModel === false ? 'border-yellow-500 focus:border-yellow-500 focus:ring-yellow-500' : ''}`} | |||
| /> | |||
| {provider && ( | |||
| {isKnownModel === true && ( | |||
| <div className="mt-1 text-xs text-green-500 flex items-center"> | |||
| <svg xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24" strokeWidth={2} stroke="currentColor" className="w-3 h-3 mr-1"> | |||
| <path strokeLinecap="round" strokeLinejoin="round" d="m4.5 12.75 6 6 9-13.5" /> | |||
| </svg> | |||
| Recognized model | |||
| </div> | |||
| )} | |||
| {isKnownModel === false && ( | |||
| <div className="mt-1 text-xs text-yellow-500 flex items-center"> | |||
| <svg xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24" strokeWidth={2} stroke="currentColor" className="w-3 h-3 mr-1"> | |||
| <path strokeLinecap="round" strokeLinejoin="round" d="m11.25 11.25.041-.02a.75.75 0 0 1 1.063.852l-.708 2.836a.75.75 0 0 0 1.063.853l.041-.021M21 12a9 9 0 1 1-18 0 9 9 0 0 1 18 0Zm-9-3.75h.008v.008H12V8.25Z" /> | |||
| </svg> | |||
| Unrecognized model. It may still work if it's new or custom. | |||
| </div> | |||
| )} | |||
There was a problem hiding this comment.
This PR adds new user-facing behavior (API key reveal toggle and real-time model recognition/warning), but there are no unit tests covering it. Please add tests (e.g., in src/tests/components/) asserting the toggle changes the API key input type and that recognized vs unrecognized models render the correct status message for a given provider.
| <button | ||
| type="button" | ||
| onClick={() => setShowApiKey(!showApiKey)} | ||
| className={`absolute inset-y-0 right-0 pr-3 flex items-center ${theme.helpText} hover:text-gray-700`} | ||
| tabIndex={-1} | ||
| > | ||
| {showApiKey ? ( | ||
| <svg xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24" strokeWidth={1.5} stroke="currentColor" className="w-5 h-5"> | ||
| <path strokeLinecap="round" strokeLinejoin="round" d="M3.98 8.223A10.477 10.477 0 0 0 1.934 12C3.226 16.338 7.244 19.5 12 19.5c.993 0 1.953-.138 2.863-.395M6.228 6.228A10.45 10.45 0 0 1 12 4.5c4.756 0 8.773 3.162 10.065 7.498a10.523 10.523 0 0 1-4.293 5.774M6.228 6.228 3 3m3.228 3.228 3.65 3.65m7.894 7.894L21 21m-3.228-3.228-3.65-3.65m0 0a3 3 0 1 0-4.243-4.243m4.242 4.242L9.88 9.88" /> | ||
| </svg> | ||
| ) : ( | ||
| <svg xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24" strokeWidth={1.5} stroke="currentColor" className="w-5 h-5"> | ||
| <path strokeLinecap="round" strokeLinejoin="round" d="M2.036 12.322a1.012 1.012 0 0 1 0-.639C3.423 7.51 7.36 4.5 12 4.5c4.638 0 8.573 3.007 9.963 7.178.07.207.07.431 0 .639C20.577 16.49 16.64 19.5 12 19.5c-4.638 0-8.573-3.007-9.963-7.178Z" /> | ||
| <path strokeLinecap="round" strokeLinejoin="round" d="M15 12a3 3 0 1 1-6 0 3 3 0 0 1 6 0Z" /> | ||
| </svg> | ||
| )} | ||
| </button> |
There was a problem hiding this comment.
The API-key visibility toggle button is not keyboard accessible (tabIndex={-1}) and has no accessible name. This prevents keyboard/screen-reader users from discovering or using the control; consider removing the negative tabIndex and adding an aria-label (and optionally aria-pressed) describing "Show/Hide API key".
| const isKnownModel = useMemo(() => { | ||
| if (!provider || !model) return null; | ||
| if (provider === 'openrouter' || provider === 'ollama' || provider === 'openai-compatible') return null; // Can't easily validate | ||
|
|
||
| const knownForProvider = KNOWN_MODELS[provider]; | ||
| if (!knownForProvider) return null; | ||
|
|
||
| return knownForProvider.includes(model); | ||
| }, [provider, model]); |
There was a problem hiding this comment.
The model-name validation is strict string equality against KNOWN_MODELS. Trailing/leading whitespace (common when copy/pasting) will always show as "Unrecognized". Consider normalizing the input for comparison (e.g., trimming) and using the normalized value for includes().
Closes #<#739>
Changes
KNOWN_MODELSconstant out of the React component scope to resolvereact-hooks/exhaustive-depswarnings.Flags
KNOWN_MODELSrecord directly in AIConfigPopup.tsx. As new models are released, this dictionary will need to be occasionally updated. However, the UI handles unrecognized models gracefully with a warning rather than blocking the user.