feat(platform): provider models search and UX polish#1693
Conversation
- Add search + empty-state to provider models table; preserve table layout and minimum height when searching with zero matches - Add provider default models panel and refactor provider edit panel - UI polish across navigation, dropdown, badges, agent/automation routes - Add modelsEmpty translations (en/fr/de)
📝 WalkthroughWalkthroughThis PR introduces a comprehensive UI update and feature enhancement across the platform. It adds trailing content support to navigation and dropdown components, refactors provider settings with a new ProviderDefaultModelsPanel component and improved model search functionality, updates automation navigation to display trigger status via a badge, integrates sign-out handling into the forced password change flow with user email and logout affordances, restructures the agent webhook section to use the PageSection action prop, and updates all translation files (English, German, French) to rename webhook terminology to worker terminology while adding new UI strings for password change, provider editing, and automation rename flows. Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
services/platform/app/components/ui/forms/date-range-picker.tsx (1)
160-179:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd translated
aria-labels to the calendar nav icon buttons.Both month navigation buttons are icon-only and currently unlabeled for assistive tech.
Proposed fix
const DateInputHeader = memo(function DateInputHeader({ date, decreaseMonth, increaseMonth, prevMonthButtonDisabled, nextMonthButtonDisabled, }: DateInputHeaderProps) { + const { t } = useT('common'); return ( <div className="mb-2 flex items-center justify-between px-1"> <Button type="button" variant="secondary" size="sm" + aria-label={t('datePicker.previousMonth')} disabled={prevMonthButtonDisabled} onClick={decreaseMonth} className="hover:bg-accent size-6 p-0" > <ChevronLeft className="text-foreground size-3.5" /> </Button> <Text>{format(date, 'MMMM yyyy')}</Text> <Button type="button" size="sm" variant="secondary" + aria-label={t('datePicker.nextMonth')} disabled={nextMonthButtonDisabled} onClick={increaseMonth} className="hover:bg-accent size-6 p-0" > <ChevronRight className="text-foreground size-3.5" /> </Button> </div> ); });As per coding guidelines, "Every icon-only button has a translated
aria-label— never hardcode English in ARIA".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/platform/app/components/ui/forms/date-range-picker.tsx` around lines 160 - 179, The two icon-only navigation Buttons (the one rendering ChevronLeft and the one rendering ChevronRight) lack accessible labels; update the Button props in date-range-picker.tsx (the Buttons using prevMonthButtonDisabled/decreaseMonth and nextMonthButtonDisabled/increaseMonth) to include translated aria-labels instead of hardcoded English, e.g. obtain a translation function (e.g. t or useTranslation) in this component and set aria-label={t('calendar.previousMonth')} for the left button and aria-label={t('calendar.nextMonth')} for the right button so screen readers can announce the controls.services/platform/app/components/ui/data-table/data-table-filters.tsx (1)
323-327:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTranslate the error fallback message.
Date filter unavailableis a user-facing literal in JSX and should be routed throught(...).Suggested adjustment
errorFallback={ <Text as="span" variant="muted"> - Date filter unavailable + {t('datePicker.unavailable')} </Text> }As per coding guidelines, "No hardcoded user-facing strings in React — always use the translation hook; a stray English literal in JSX is a bug".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/platform/app/components/ui/data-table/data-table-filters.tsx` around lines 323 - 327, The JSX errorFallback is using a hardcoded user-facing string "Date filter unavailable"; update the component to use the translation hook instead (e.g., call the existing t(...) from your translations hook in this file) and replace the literal in the Text element with t('dateFilterUnavailable') or the appropriate translation key; ensure the translation hook is imported/initialized in the same component (data-table-filters) so the Text as="span" variant="muted" renders the localized string.services/platform/app/routes/dashboard/$id/settings/providers/$providerName.tsx (1)
747-767:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't leave add mode open after a partial save failure.
If
saveConfigsucceeds andsaveSecret.mutateAsyncfails, the dialog stays open witheditingIndex === null. Retrying will rebuildupdatedModelsas[...]config.models, model], so the same model can be appended again after a transient secret-save error. Either make the config+secret update atomic, or transition out of add mode as soon as the config write succeeds.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/platform/app/routes/dashboard/`$id/settings/providers/$providerName.tsx around lines 747 - 767, After saveConfig succeeds you must exit "add" mode immediately to avoid re-appending the same model if saveSecret.mutateAsync fails: inside the try block after await saveConfig({ models: updatedModels }), if editingIndex === null compute const newIndex = updatedModels.findIndex(m => m.id === form.id) and call setEditingIndex(newIndex) (or setEditingIndex(-1) / appropriate non-null value used by this component) before calling saveSecret.mutateAsync; this ensures the UI is no longer in add mode even if the secret save later fails. Make this change in the same block that calls saveConfig and saveSecret.mutateAsync (referencing saveConfig, saveSecret.mutateAsync, updatedModels, form.id, modelKeyAction, editingIndex, setEditingIndex, setSavingSecret).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@services/platform/app/components/ui/data-table/data-table-filters.tsx`:
- Around line 157-158: In the DataTableFilters component
(data-table-filters.tsx) replace the invalid Tailwind utility "min-w-auto" with
"min-w-0" on the flex container elements (the divs that currently have className
values containing "flex min-w-auto ...") so flex children can shrink correctly;
update both occurrences (the outer container with "flex min-w-auto flex-col ..."
and the inner one with "flex min-w-auto items-center ...") to use "min-w-0"
instead.
In
`@services/platform/app/features/automations/components/automation-navigation.test.tsx`:
- Around line 39-46: Add behavioral assertions around the mocked
useWorkflowActivity hook: in automation-navigation.test.tsx add two tests (or
parametrize one) that render the AutomationNavigation component while mocking
useWorkflowActivity to return hasActiveTrigger: false/true respectively, then
assert the triggers badge visibility—when hasActiveTrigger is false expect the
badge element (e.g., test id or label used for triggers badge) to be
present/visible, and when hasActiveTrigger is true expect that badge to be
absent/not found; update or create per-test mocks for useWorkflowActivity
(checking activeTriggers/totalTriggers if needed) so the new navigation logic is
actually verified.
In
`@services/platform/app/features/automations/components/automation-navigation.tsx`:
- Around line 68-72: The UI renders the “inactive” badge directly from
hasActiveTrigger before activity state loads; update the rendering logic that
uses hasActiveTrigger (from useWorkflowActivity(organizationId, workflowSlug))
to also check the isLoading flag returned by useWorkflowActivity and only
decide/display an inactive badge once isLoading is false (i.e., show a
loading/neutral state while isLoading is true), and apply the same guard to the
other badge usage around the block referenced at 91-95 so no false “inactive”
flicker appears.
In
`@services/platform/app/features/settings/providers/components/provider-default-models-panel.tsx`:
- Around line 68-74: The isDirty calculation treats unset local defaults
(undefined) as different from saved defaults using NONE_VALUE, causing false
positives; update the comparison so both sides are normalized the same way
(e.g., coerce both the local value and the saved value with nullish coalescing
to NONE_VALUE) before comparing. Specifically, change the isDirty logic that
references defaults.chat, defaults.vision, defaults.embedding,
defaults.transcription and data.config.defaults?.* so each pair is compared as
(defaults.<key> ?? NONE_VALUE) !== (data.config.defaults?.<key> ?? NONE_VALUE),
ensuring unset values no longer register as dirty.
In
`@services/platform/app/routes/dashboard/`$id/settings/providers/$providerName.tsx:
- Around line 895-899: The TableRow is currently the only edit affordance
(onClick={() => openEditDialog(index)} on TableRow) which is mouse-only; add an
explicit interactive element (e.g., a <button> or <a> styled like an icon/link)
inside the row that calls openEditDialog(index) so keyboard and assistive-tech
users can open the edit sheet, ensure the new element is focusable, has an
accessible name (aria-label or visible text), meets the 24×24/44×44 touch target
guideline, and keep the TableRow click optional or remove it to avoid duplicate
activation.
- Around line 863-879: The table defined by Table/TableHeader/TableRow/TableHead
lacks an accessible caption and proper semantics; add an sr-only <caption>
describing the models list (e.g., "Models for {providerName}") inside the Table,
ensure each TableHead element (the ones in this diff) includes scope="col", and
make the selectable row rendering (the rows created where selection state
exists) include aria-selected set to boolean selection state so screen readers
can identify selected rows.
- Around line 874-876: The TableHead header "Cost / 1M tokens" is hardcoded;
replace it with the translation hook (useT or t) so it’s localized (e.g., change
the literal inside the TableHead in the providers settings component to
{t('providers.costPerMillionTokens')}), add the matching key
"providers.costPerMillionTokens" to the i18n resource files for all languages,
and ensure useT/t is imported and in-scope in the component that renders the
TableHead.
- Around line 332-339: These text-only buttons lack keyboard-visible focus
styles; update the button rendered for the edit action (the element using
onClick={onEdit} with editLabel and the Pencil icon) and the similar
test-connection and model-key action buttons to either use the shared Button
component or add explicit focus-visible styling (e.g., visible focus ring,
offset and contrast) and ensure minimum touch target sizing; specifically,
augment the button className to include a focus-visible ring and offset (so
keyboard users see a >=3:1 contrast ring) and confirm the interactive area meets
the 24×24 CSS pixel (44×44 mobile) guideline for the actions referenced in this
file.
In `@services/platform/messages/en.json`:
- Line 600: Update the metadata copy to match the new “Workers” terminology:
change the values for metadata.agentWebhook.title and
metadata.agentWebhook.description to use “Workers” (or equivalent phrasing)
instead of “webhook” so they align with the navigation label “Workers”; find and
edit the JSON keys metadata.agentWebhook.title and
metadata.agentWebhook.description to update their strings consistently across
the file.
---
Outside diff comments:
In `@services/platform/app/components/ui/data-table/data-table-filters.tsx`:
- Around line 323-327: The JSX errorFallback is using a hardcoded user-facing
string "Date filter unavailable"; update the component to use the translation
hook instead (e.g., call the existing t(...) from your translations hook in this
file) and replace the literal in the Text element with
t('dateFilterUnavailable') or the appropriate translation key; ensure the
translation hook is imported/initialized in the same component
(data-table-filters) so the Text as="span" variant="muted" renders the localized
string.
In `@services/platform/app/components/ui/forms/date-range-picker.tsx`:
- Around line 160-179: The two icon-only navigation Buttons (the one rendering
ChevronLeft and the one rendering ChevronRight) lack accessible labels; update
the Button props in date-range-picker.tsx (the Buttons using
prevMonthButtonDisabled/decreaseMonth and nextMonthButtonDisabled/increaseMonth)
to include translated aria-labels instead of hardcoded English, e.g. obtain a
translation function (e.g. t or useTranslation) in this component and set
aria-label={t('calendar.previousMonth')} for the left button and
aria-label={t('calendar.nextMonth')} for the right button so screen readers can
announce the controls.
In
`@services/platform/app/routes/dashboard/`$id/settings/providers/$providerName.tsx:
- Around line 747-767: After saveConfig succeeds you must exit "add" mode
immediately to avoid re-appending the same model if saveSecret.mutateAsync
fails: inside the try block after await saveConfig({ models: updatedModels }),
if editingIndex === null compute const newIndex = updatedModels.findIndex(m =>
m.id === form.id) and call setEditingIndex(newIndex) (or setEditingIndex(-1) /
appropriate non-null value used by this component) before calling
saveSecret.mutateAsync; this ensures the UI is no longer in add mode even if the
secret save later fails. Make this change in the same block that calls
saveConfig and saveSecret.mutateAsync (referencing saveConfig,
saveSecret.mutateAsync, updatedModels, form.id, modelKeyAction, editingIndex,
setEditingIndex, setSavingSecret).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1c45dbe2-7a2f-441e-aaa0-42ec73306027
📒 Files selected for processing (21)
services/platform/app/components/ui/data-table/data-table-filters.tsxservices/platform/app/components/ui/forms/date-range-picker.tsxservices/platform/app/components/ui/navigation/navigation-menu.tsxservices/platform/app/components/ui/navigation/navigation.tsxservices/platform/app/components/ui/navigation/tab-navigation.tsxservices/platform/app/components/ui/navigation/tabs.tsxservices/platform/app/components/ui/overlays/dropdown-menu.tsxservices/platform/app/components/user-button.tsxservices/platform/app/features/agents/components/agent-webhook-section.tsxservices/platform/app/features/automations/components/automation-navigation.test.tsxservices/platform/app/features/automations/components/automation-navigation.tsxservices/platform/app/features/automations/components/automation-rename-dialog.tsxservices/platform/app/features/settings/providers/components/provider-default-models-panel.tsxservices/platform/app/features/settings/providers/components/provider-edit-panel.tsxservices/platform/app/routes/dashboard/$id/agents/$agentId.tsxservices/platform/app/routes/dashboard/$id/automations/$amId.tsxservices/platform/app/routes/dashboard/$id/settings/providers/$providerName.tsxservices/platform/app/routes/forced-change-password.$id.tsxservices/platform/messages/de.jsonservices/platform/messages/en.jsonservices/platform/messages/fr.json
💤 Files with no reviewable changes (1)
- services/platform/app/routes/dashboard/$id/automations/$amId.tsx
| <div className="flex min-w-auto flex-col items-start gap-3 sm:flex-1 sm:flex-row sm:flex-wrap sm:items-center"> | ||
| <div className="flex min-w-auto items-center gap-3"> |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does Tailwind CSS v4 include a min-w-auto utility class? If not, what utility should be used for flex children that need to shrink (min-w-0)?
💡 Result:
No. Tailwind CSS provides the min-width utilities (min-w-*), but there is not a min-w-auto utility class. The built-in min-width docs describe utilities like min-w-0, min-w-full, fixed scale values, percentage values, and arbitrary values via min-w-[...]/min-w-(...)—not min-w-auto [1]. For flex children that need to be allowed to shrink, use min-w-0 (i.e., add min-w-0 to the flex child). This is the common pattern to ensure the item can shrink when its content would otherwise force a larger intrinsic minimum size [2][3].
Citations:
- 1: https://tailwindcss.com/docs/min-width
- 2: https://runebook.dev/en/docs/tailwindcss/flex-shrink
- 3: .flex-1 (and possibly others) requires min-width: 0 tailwindlabs/tailwindcss#809
Replace min-w-auto with min-w-0 for flex children that need to shrink below their content width.
min-w-auto is not a valid Tailwind utility and will be ignored; these lines silently receive no min-width constraint. Use min-w-0 to allow flex children to shrink.
Suggested adjustment
- <div className="flex min-w-auto flex-col items-start gap-3 sm:flex-1 sm:flex-row sm:flex-wrap sm:items-center">
- <div className="flex min-w-auto items-center gap-3">
+ <div className="flex min-w-0 flex-col items-start gap-3 sm:flex-1 sm:flex-row sm:flex-wrap sm:items-center">
+ <div className="flex min-w-0 items-center gap-3">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div className="flex min-w-auto flex-col items-start gap-3 sm:flex-1 sm:flex-row sm:flex-wrap sm:items-center"> | |
| <div className="flex min-w-auto items-center gap-3"> | |
| <div className="flex min-w-0 flex-col items-start gap-3 sm:flex-1 sm:flex-row sm:flex-wrap sm:items-center"> | |
| <div className="flex min-w-0 items-center gap-3"> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/platform/app/components/ui/data-table/data-table-filters.tsx` around
lines 157 - 158, In the DataTableFilters component (data-table-filters.tsx)
replace the invalid Tailwind utility "min-w-auto" with "min-w-0" on the flex
container elements (the divs that currently have className values containing
"flex min-w-auto ...") so flex children can shrink correctly; update both
occurrences (the outer container with "flex min-w-auto flex-col ..." and the
inner one with "flex min-w-auto items-center ...") to use "min-w-0" instead.
| vi.mock('../triggers/hooks/queries', () => ({ | ||
| useWorkflowActivity: () => ({ | ||
| hasActiveTrigger: false, | ||
| activeTriggers: 0, | ||
| totalTriggers: 0, | ||
| isLoading: false, | ||
| }), | ||
| })); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Add behavioral assertions for the newly mocked trigger state.
This mock introduces inactive-state behavior, but current tests only run axe checks. Add assertions for badge visibility when inactive, and absence when active, so the new navigation logic is actually verified.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@services/platform/app/features/automations/components/automation-navigation.test.tsx`
around lines 39 - 46, Add behavioral assertions around the mocked
useWorkflowActivity hook: in automation-navigation.test.tsx add two tests (or
parametrize one) that render the AutomationNavigation component while mocking
useWorkflowActivity to return hasActiveTrigger: false/true respectively, then
assert the triggers badge visibility—when hasActiveTrigger is false expect the
badge element (e.g., test id or label used for triggers badge) to be
present/visible, and when hasActiveTrigger is true expect that badge to be
absent/not found; update or create per-test mocks for useWorkflowActivity
(checking activeTriggers/totalTriggers if needed) so the new navigation logic is
actually verified.
| const { hasActiveTrigger } = useWorkflowActivity( | ||
| organizationId, | ||
| workflowSlug, | ||
| ); | ||
|
|
There was a problem hiding this comment.
Guard inactive status rendering until activity state is loaded.
hasActiveTrigger is used directly, so the UI can briefly show an incorrect “inactive” badge while trigger activity is still loading. Gate badge rendering on isLoading to avoid false status flicker.
💡 Suggested fix
- const { hasActiveTrigger } = useWorkflowActivity(
+ const { hasActiveTrigger, isLoading } = useWorkflowActivity(
organizationId,
workflowSlug,
);
...
- trailing: !hasActiveTrigger ? (
+ trailing: !isLoading && !hasActiveTrigger ? (
<span className="border-border bg-background text-foreground ml-2 inline-flex items-center rounded-md border px-1.5 py-0.5 text-xs leading-4 font-medium">
{tCommon('status.inactive')}
</span>
) : null,Also applies to: 91-95
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@services/platform/app/features/automations/components/automation-navigation.tsx`
around lines 68 - 72, The UI renders the “inactive” badge directly from
hasActiveTrigger before activity state loads; update the rendering logic that
uses hasActiveTrigger (from useWorkflowActivity(organizationId, workflowSlug))
to also check the isLoading flag returned by useWorkflowActivity and only
decide/display an inactive badge once isLoading is false (i.e., show a
loading/neutral state while isLoading is true), and apply the same guard to the
other badge usage around the block referenced at 91-95 so no false “inactive”
flicker appears.
| const isDirty = | ||
| !!data?.ok && | ||
| (defaults.chat !== (data.config.defaults?.chat ?? NONE_VALUE) || | ||
| defaults.vision !== (data.config.defaults?.vision ?? NONE_VALUE) || | ||
| defaults.embedding !== (data.config.defaults?.embedding ?? NONE_VALUE) || | ||
| defaults.transcription !== | ||
| (data.config.defaults?.transcription ?? NONE_VALUE)); |
There was a problem hiding this comment.
isDirty is incorrectly true when defaults are unset.
Unset defaults compare as undefined on the left and __none__ on the right, so the dialog can appear dirty without any user edits.
🔧 Suggested fix
const isDirty =
!!data?.ok &&
- (defaults.chat !== (data.config.defaults?.chat ?? NONE_VALUE) ||
- defaults.vision !== (data.config.defaults?.vision ?? NONE_VALUE) ||
- defaults.embedding !== (data.config.defaults?.embedding ?? NONE_VALUE) ||
- defaults.transcription !==
+ ((defaults.chat ?? NONE_VALUE) !== (data.config.defaults?.chat ?? NONE_VALUE) ||
+ (defaults.vision ?? NONE_VALUE) !== (data.config.defaults?.vision ?? NONE_VALUE) ||
+ (defaults.embedding ?? NONE_VALUE) !== (data.config.defaults?.embedding ?? NONE_VALUE) ||
+ (defaults.transcription ?? NONE_VALUE) !==
(data.config.defaults?.transcription ?? NONE_VALUE));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const isDirty = | |
| !!data?.ok && | |
| (defaults.chat !== (data.config.defaults?.chat ?? NONE_VALUE) || | |
| defaults.vision !== (data.config.defaults?.vision ?? NONE_VALUE) || | |
| defaults.embedding !== (data.config.defaults?.embedding ?? NONE_VALUE) || | |
| defaults.transcription !== | |
| (data.config.defaults?.transcription ?? NONE_VALUE)); | |
| const isDirty = | |
| !!data?.ok && | |
| ((defaults.chat ?? NONE_VALUE) !== (data.config.defaults?.chat ?? NONE_VALUE) || | |
| (defaults.vision ?? NONE_VALUE) !== (data.config.defaults?.vision ?? NONE_VALUE) || | |
| (defaults.embedding ?? NONE_VALUE) !== (data.config.defaults?.embedding ?? NONE_VALUE) || | |
| (defaults.transcription ?? NONE_VALUE) !== | |
| (data.config.defaults?.transcription ?? NONE_VALUE)); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@services/platform/app/features/settings/providers/components/provider-default-models-panel.tsx`
around lines 68 - 74, The isDirty calculation treats unset local defaults
(undefined) as different from saved defaults using NONE_VALUE, causing false
positives; update the comparison so both sides are normalized the same way
(e.g., coerce both the local value and the saved value with nullish coalescing
to NONE_VALUE) before comparing. Specifically, change the isDirty logic that
references defaults.chat, defaults.vision, defaults.embedding,
defaults.transcription and data.config.defaults?.* so each pair is compared as
(defaults.<key> ?? NONE_VALUE) !== (data.config.defaults?.<key> ?? NONE_VALUE),
ensuring unset values no longer register as dirty.
| <button | ||
| type="button" | ||
| onClick={onEdit} | ||
| className="text-muted-foreground hover:text-foreground flex shrink-0 items-center gap-1.5 text-[13px] font-medium" | ||
| > | ||
| <Pencil className="size-3.5" /> | ||
| {editLabel} | ||
| </button> |
There was a problem hiding this comment.
Add visible focus styling to the new text buttons.
These actions only define hover styles. If the reset removes the browser default outline, keyboard users lose their position on the section edit, test connection, and model-key action buttons. Prefer the shared button components or add explicit focus-visible treatment here.
As per coding guidelines, "Everything interactive is keyboard-reachable — test with Tab and Shift+Tab; focus rings stay visible with ≥3:1 contrast; focus traps only in modals and return to trigger on close; minimum 24×24 CSS pixel touch target (44×44 mobile)".
Also applies to: 517-524, 1187-1213
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@services/platform/app/routes/dashboard/`$id/settings/providers/$providerName.tsx
around lines 332 - 339, These text-only buttons lack keyboard-visible focus
styles; update the button rendered for the edit action (the element using
onClick={onEdit} with editLabel and the Pencil icon) and the similar
test-connection and model-key action buttons to either use the shared Button
component or add explicit focus-visible styling (e.g., visible focus ring,
offset and contrast) and ensure minimum touch target sizing; specifically,
augment the button className to include a focus-visible ring and offset (so
keyboard users see a >=3:1 contrast ring) and confirm the interactive area meets
the 24×24 CSS pixel (44×44 mobile) guideline for the actions referenced in this
file.
| <Table> | ||
| <TableHeader> | ||
| <TableRow> | ||
| <TableHead>{t('providers.modelId')}</TableHead> | ||
| <TableHead className="w-[200px]"> | ||
| {t('providers.displayName')} | ||
| </TableHead> | ||
| <TableHead>{t('providers.description_field')}</TableHead> | ||
| <TableHead className="w-[200px]"> | ||
| {t('providers.tags')} | ||
| </TableHead> | ||
| <TableHead className="w-[140px] text-right"> | ||
| Cost / 1M tokens | ||
| </TableHead> | ||
| <TableHead className="w-11" /> | ||
| </TableRow> | ||
| </TableHeader> |
There was a problem hiding this comment.
Add a caption to the models table.
The new table has headers, but no <caption>, so screen-reader users do not get an accessible name or context for what this table represents. An sr-only caption is enough.
As per coding guidelines, "Tables must have <caption> (sr-only is fine), scope="col" on every <th>, and aria-selected on selected rows".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@services/platform/app/routes/dashboard/`$id/settings/providers/$providerName.tsx
around lines 863 - 879, The table defined by
Table/TableHeader/TableRow/TableHead lacks an accessible caption and proper
semantics; add an sr-only <caption> describing the models list (e.g., "Models
for {providerName}") inside the Table, ensure each TableHead element (the ones
in this diff) includes scope="col", and make the selectable row rendering (the
rows created where selection state exists) include aria-selected set to boolean
selection state so screen readers can identify selected rows.
| <TableHead className="w-[140px] text-right"> | ||
| Cost / 1M tokens | ||
| </TableHead> |
There was a problem hiding this comment.
Localize the new cost column header.
Cost / 1M tokens bypasses useT, so this column stays English in French and German while the rest of the table is translated.
As per coding guidelines, "No hardcoded user-facing strings in React — always use the translation hook; a stray English literal in JSX is a bug".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@services/platform/app/routes/dashboard/`$id/settings/providers/$providerName.tsx
around lines 874 - 876, The TableHead header "Cost / 1M tokens" is hardcoded;
replace it with the translation hook (useT or t) so it’s localized (e.g., change
the literal inside the TableHead in the providers settings component to
{t('providers.costPerMillionTokens')}), add the matching key
"providers.costPerMillionTokens" to the i18n resource files for all languages,
and ensure useT/t is imported and in-scope in the component that renders the
TableHead.
| <TableRow | ||
| key={index} | ||
| className="cursor-pointer" | ||
| onClick={() => openEditDialog(index)} | ||
| > |
There was a problem hiding this comment.
Don't make the table row the only edit affordance.
Line 898 turns <TableRow> into a mouse-only trigger. Table rows are not keyboard-operable by default, so keyboard and assistive-tech users currently have no equivalent way to open the edit sheet. Add an explicit button/link inside the row instead of relying on row clicks.
As per coding guidelines, "Use real HTML elements — <button>, <nav>, <main>, <header>, <footer>, <article>, <section>; <div onClick> is not a button" and "Everything interactive is keyboard-reachable — test with Tab and Shift+Tab; focus rings stay visible with ≥3:1 contrast; focus traps only in modals and return to trigger on close; minimum 24×24 CSS pixel touch target (44×44 mobile)".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@services/platform/app/routes/dashboard/`$id/settings/providers/$providerName.tsx
around lines 895 - 899, The TableRow is currently the only edit affordance
(onClick={() => openEditDialog(index)} on TableRow) which is mouse-only; add an
explicit interactive element (e.g., a <button> or <a> styled like an icon/link)
inside the row that calls openEditDialog(index) so keyboard and assistive-tech
users can open the edit sheet, ensure the new element is focusable, has an
accessible name (aria-label or visible text), meets the 24×24/44×44 touch target
guideline, and keep the TableRow click optional or remove it to avoid duplicate
activation.
| "tools": "Tools", | ||
| "knowledge": "Knowledge", | ||
| "webhook": "Webhook", | ||
| "webhook": "Workers", |
There was a problem hiding this comment.
Align metadata copy with the new “Workers” terminology.
Line 600 updates the navigation label to “Workers”, but metadata.agentWebhook.title and metadata.agentWebhook.description still say “webhook”, which creates a visible naming mismatch.
Proposed copy update
- "agentWebhook": {
- "title": "Agent webhook",
- "description": "Manage webhook URLs for your agent."
- },
+ "agentWebhook": {
+ "title": "Agent workers",
+ "description": "Manage worker URLs for your agent."
+ },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/platform/messages/en.json` at line 600, Update the metadata copy to
match the new “Workers” terminology: change the values for
metadata.agentWebhook.title and metadata.agentWebhook.description to use
“Workers” (or equivalent phrasing) instead of “webhook” so they align with the
navigation label “Workers”; find and edit the JSON keys
metadata.agentWebhook.title and metadata.agentWebhook.description to update
their strings consistently across the file.
- Replace invalid min-w-auto Tailwind class with min-w-0 in data-table-filters so flex children can shrink as intended. - Localize "Cost / 1M tokens" header in the provider models table; add providers.costPerMillionTokens to en/fr/de. - Fix isDirty false-positive in provider-default-models-panel by normalizing both sides with NONE_VALUE before comparing. - Gate the automation triggers "inactive" badge on activity isLoading to prevent a brief incorrect inactive state on first render. - Align metadata.agentWebhook copy with the new "Workers" terminology in en/fr/de.
1 participant
Summary
providers.modelsEmptytranslation block to en/fr/de.Test plan
Summary by CodeRabbit
New Features
Enhancements
Updates