feat(platform): add StatGrid component, improve icons and UI components#592
Conversation
Introduce StatGrid for structured data display using definition lists, refactor icon components to use responsive sizing with cn(), improve checkbox layout with description alignment, refactor detail dialogs to use StatGrid, and update Storybook stories across UI components.
Greptile SummaryThis PR introduces a new Key Changes:
Benefits:
Confidence Score: 5/5
|
| Filename | Overview |
|---|---|
| services/platform/app/components/ui/data-display/stat-grid.tsx | New StatGrid component using definition lists with responsive column layouts - clean implementation |
| services/platform/app/components/ui/data-display/stat-item.tsx | StatItem component using semantic dt/dd elements with proper colSpan support |
| services/platform/app/components/ui/forms/checkbox.tsx | Improved checkbox layout - description now aligns properly with label using flex column |
| services/platform/app/features/approvals/components/approval-detail-dialog.tsx | Refactored to use StatGrid for displaying approval metadata - cleaner structured data display |
| services/platform/app/features/chat/components/message-info-dialog.tsx | Uses StatGrid for token usage and performance metrics display |
| services/platform/app/features/products/components/product-view-dialog.tsx | Product details now displayed with StatGrid for consistent UI |
| services/platform/app/features/websites/components/website-view-dialog.tsx | Website details refactored to use StatGrid with colSpan support for longer fields |
Class Diagram
%%{init: {'theme': 'neutral'}}%%
classDiagram
class StatGrid {
+items: StatGridItem[]
+cols: 1|2|3|4
+className: string
+render() dl
}
class StatItem {
+label: string
+children: ReactNode
+colSpan: 1|2
+className: string
+render() div with dt/dd
}
class StatGridItem {
+label: string
+value: ReactNode
+colSpan: 1|2
}
class ApprovalDetailDialog {
+uses StatGrid for metadata
}
class MessageInfoDialog {
+uses StatGrid for tokens
+uses StatGrid for performance
}
class ProductViewDialog {
+uses StatGrid for details
}
class WebsiteViewDialog {
+uses StatGrid for info
}
class IntegrationDetails {
+uses StatGrid for SQL config
}
StatGrid --> StatItem : renders
StatGrid ..> StatGridItem : uses
ApprovalDetailDialog --> StatGrid : uses
MessageInfoDialog --> StatGrid : uses
ProductViewDialog --> StatGrid : uses
WebsiteViewDialog --> StatGrid : uses
IntegrationDetails --> StatGrid : uses
Last reviewed commit: f2f4a42
📝 WalkthroughWalkthroughThis PR refactors multiple UI components and icon files to improve styling and layout patterns. It updates approximately 12 icon components (CirculyIcon, EnterKeyIcon, GmailIcon, LocaleIcon, MicrosoftIcon, OneDriveIcon, OutlookIcon, ProtelIcon, SharePointIcon, ShopifyIcon, WebsiteIcon) to use a Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
services/platform/app/components/ui/layout/separator.stories.tsx (1)
48-59: 🧹 Nitpick | 🔵 TrivialArgs-based migration is correct; consider applying consistently to other stories.
The
Horizontalstory correctly adopts the args-based pattern, enabling dynamic prop control in Storybook. For consistency across the file, consider also migratingVertical(line 61) andInToolbar(line 96) stories, which still use hardcodedorientation="vertical"props.♻️ Suggested refactor for Vertical story
export const Vertical: Story = { + args: { + orientation: 'vertical', + }, - render: () => ( + render: (args) => ( <div className="flex h-8 items-center gap-4"> <span className="text-sm">Item 1</span> - <Separator orientation="vertical" /> + <Separator {...args} /> <span className="text-sm">Item 2</span> - <Separator orientation="vertical" /> + <Separator {...args} /> <span className="text-sm">Item 3</span> </div> ), };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform/app/components/ui/layout/separator.stories.tsx` around lines 48 - 59, The Horizontal story now uses args to control Separator props; update the other stories (Vertical and InToolbar) to the same args-based pattern by moving the hardcoded orientation="vertical" into their args objects and using render: (args) => <Separator {...args} /> (or wrap in the same container JSX used by those stories), so both Vertical and InToolbar accept dynamic props via args while still rendering the Separator component.services/platform/app/components/icons/outlook-icon.tsx (1)
1-9: 🧹 Nitpick | 🔵 TrivialLGTM! Consider adding
aria-labelfor consistency.The
cnutility integration is correct. However, this icon lacks anaria-labelattribute while other icons in this PR (e.g.,LocaleIcon,EnterKeyIcon,CirculyIcon,ProtelIcon) include one. For consistency and accessibility, consider addingaria-label="Outlook icon"oraria-hiddenif purely decorative.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform/app/components/icons/outlook-icon.tsx` around lines 1 - 9, The OutlookIcon component's <svg> (in the OutlookIcon function) is missing an accessibility attribute for consistency; update the SVG element inside OutlookIcon to include either aria-label="Outlook icon" (if it conveys meaning) or aria-hidden="true" (if decorative) while keeping the existing className usage via cn and props signature unchanged so other icons remain consistent.services/platform/app/components/icons/gmail-icon.tsx (1)
1-11: 🧹 Nitpick | 🔵 TrivialLGTM! Consider adding accessibility attribute for consistency.
The
cnutility integration is correct. Similar toOutlookIcon, this component lacks anaria-labeloraria-hiddenattribute while other icons in this PR include them. Consider adding one for consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform/app/components/icons/gmail-icon.tsx` around lines 1 - 11, GmailIcon is missing an accessibility attribute—match other icons (like OutlookIcon) by adding either an aria-label when the icon is meaningful or aria-hidden="true" when decorative; update the GmailIcon JSX root <svg> to include the chosen attribute (or accept and forward an ariaLabel prop) so accessibility usage is consistent with the other icon components and the cn utility usage remains unchanged.services/platform/app/features/websites/components/website-view-dialog.tsx (1)
104-120:⚠️ Potential issue | 🟡 MinorLocalize hardcoded StatGrid labels.
Line 107 (
'domain') and Line 119 ('apiEndpoint') bypass i18n and will render untranslated strings in localized UIs.💡 Suggested patch
- label: 'domain', + label: t('viewDialog.domain'), @@ - label: 'apiEndpoint', + label: t('viewDialog.apiEndpoint'),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform/app/features/websites/components/website-view-dialog.tsx` around lines 104 - 120, The StatGrid items currently contain hardcoded label keys ('domain' and 'apiEndpoint') in the items array inside website-view-dialog.tsx; replace those literal strings with calls to the app's i18n helper (e.g., useTranslation/useIntl and t(...) or formatMessage(...)) so the labels are localized before being passed into StatGrid, updating the items construction in the WebsiteViewDialog component to use translated values for the 'domain' and 'apiEndpoint' labels.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@services/platform/app/components/icons/onedrive-icon.tsx`:
- Line 5: Add a non-empty <title> element for the SVG in the OnedriveIcon
component to satisfy the lint rule; insert a unique id (e.g., titleId) and
render <title id={titleId}>OneDrive</title> inside the <svg>, then add
aria-labelledby={titleId} (or role="img") on the same <svg> element instead of
aria-hidden so screen readers can read the title; update the component props to
generate/accept titleId if needed and keep existing className/cn usage intact.
In `@services/platform/app/components/ui/data-display/selectable-row.stories.tsx`:
- Around line 52-55: The stories render a block-level <Text> inside the
SelectableRow button causing invalid semantics; update the story args so the
children use an inline element (e.g., change the Text usage to Text with
as="span" or another phrasing element) for both the unselected and selected
story entries (look for the SelectableRow story args where children is currently
<Text>Unselected row</Text> and the other case at the following story with
children <Text>Selected row</Text> and replace them accordingly).
In `@services/platform/app/components/ui/data-display/stat-grid.stories.tsx`:
- Around line 90-121: The story "WithColSpan" currently relies on an implicit
2-column default but doesn't declare it; update the Story object (WithColSpan)
to explicitly set the grid column count so the colSpan: 2 example is
self-documenting — add a cols: 2 (or the appropriate prop name your StatGrid
expects) to the args alongside items so the example clearly shows the full-width
behavior for a 2-column grid.
In `@services/platform/app/components/ui/data-display/stat-grid.tsx`:
- Around line 42-46: The current map uses item.label as the React key which can
collide; update the StatGridItem type to include an optional unique id (e.g.,
id?: string) and change the mapping in StatGrid component to use a stable key
fallback like item.id ?? item.label ?? index (use the array index only as last
resort). Ensure StatItem rendering still receives label and colSpan props and
keep the key logic inside the items.map callback so duplicate labels no longer
cause key collisions.
In `@services/platform/app/components/ui/feedback/progress.stories.tsx`:
- Around line 150-163: The story hardcodes the label and percentage so Storybook
controls don't update the surrounding text; update UploadExample's render to
read from the story args (use args.label and args.value) and render the label
and percentage dynamically alongside <Progress {...args} /> so changing
args.value or args.label in controls updates both the bar and the displayed
text.
In `@services/platform/app/components/ui/feedback/spinner.stories.tsx`:
- Around line 101-107: The button in the story rendering the Spinner lacks an
explicit type and therefore defaults to type="submit"; update the JSX for the
button (the element wrapping <Spinner {...args} /> and "Submitting...") to
include an explicit type attribute (e.g., type="button") to prevent accidental
form submissions and follow accessibility/best-practice guidelines.
In `@services/platform/app/components/ui/overlays/dropdown-menu.stories.tsx`:
- Around line 207-214: Replace the semantically incorrect LogOut icon used for
the destructive "Delete" menu item: add Trash2 to the lucide-react import list
and change the menu item icon reference from LogOut to Trash2 in the dropdown
menu item (the object with type: 'item', label: 'Delete', destructive: true, and
onClick). Ensure the import and the menu item’s icon property both reference
Trash2 so the delete action uses a trash icon.
In `@services/platform/app/components/ui/typography/heading.stories.tsx`:
- Around line 64-69: The story sets args.truncate = true but doesn’t constrain
the container width so ellipsis won’t appear; update the Heading story that
defines the args object (the one with level, size, truncate, children) to either
wrap the rendered Heading in a fixed/max-width container (similar to the
Truncated story) or remove the truncate prop from args so the story accurately
demonstrates behavior.
In `@services/platform/app/components/ui/typography/text.stories.tsx`:
- Line 90: The story uses a sample string that looks like a production secret
("sk_live_abc123def456") which can trigger secret scanners; update the story
args (the object assigned to args in the Text story) to use a neutral example
token (e.g., "pk_test_example_123", "example_token_ABC123", or simply
"example-token") instead of any "sk_live_" prefixed value so the Storybook
example remains clear without resembling real secrets — locate and edit the args
entry that currently sets as: 'span', variant: 'code', children:
'sk_live_abc123def456' and replace the children value with a harmless example
token.
In `@services/platform/app/features/chat/components/message-info-dialog.tsx`:
- Around line 200-217: The IIFE that builds perfItems in message-info-dialog.tsx
should be extracted into a memoized value using useMemo to match the token items
pattern; replace the IIFE with a const perfItems = useMemo(() => { /* build
array from metadata.durationMs and metadata.timeToFirstTokenMs, filter out
null/undefined, map to {label: t(`messageInfo.${key}`), value:
<Text>{(value/1000).toFixed(2)}s</Text>} */ }, [metadata.durationMs,
metadata.timeToFirstTokenMs, t]) and render the Field/StatGrid conditionally
based on perfItems.length > 0 as before.
- Around line 157-198: Extract the IIFE that builds tokenItems into a top-level
constant (preferably using useMemo) to improve readability and memoize
calculations: create a const tokenItems = useMemo(() => { ... }, [metadata, t,
tCommon, locale]) that constructs the same array (including the conditional
ContextWindowToken entry and the filtered/mapped token tuples) and then replace
the inline IIFE in the JSX with a simple conditional render like
tokenItems.length > 0 && (<Field ...><StatGrid ... items={tokenItems}
/></Field>). Ensure you reference the same symbols (metadata,
ContextWindowToken, StatGrid, Field, t, tCommon, locale, formatNumber) and keep
the same filtering predicate ([number | undefined, string] => entry[0] != null
&& entry[0] > 0) so behavior is unchanged.
In
`@services/platform/app/features/customers/components/customer-information.tsx`:
- Around line 49-51: The UI currently hardcodes a fallback of 'en' when
customer.locale is missing in the customer-information component; update the
locale cell (the object with label: t('labels.locale') and value) to avoid
showing 'en' as a presumed locale and instead render a proper "not available"
indicator or the true system default: replace the hardcoded string usage of 'en'
with a translation placeholder (e.g., t('notAvailable')) or a conditional that
displays t('notAvailable') when customer.locale is falsy (or use the configured
system default source if available), updating the value expression where
customer.locale is referenced.
In
`@services/platform/app/features/settings/integrations/components/integration-details.tsx`:
- Around line 348-357: The displayed readOnly strings are hardcoded "Yes"/"No";
replace them with translated values by calling the i18n function (e.g., use
t('common.yes') and t('common.no')) where the value is set for
integration.sqlConnectionConfig.readOnly inside the integration-details.tsx
component so the label value becomes integration.sqlConnectionConfig.readOnly ?
t('common.yes') : t('common.no'); ensure you import/use the existing t function
already used in this file.
In
`@services/platform/app/features/settings/integrations/components/integration-manage/integration-active-view.tsx`:
- Around line 88-120: Replace the hardcoded/internal labels ('apiKey',
'accessToken', 'domain', 'apiEndpoint') used when building the displayed items
with user-facing, localized strings via the translation function (t). Update the
label entries in the arrays constructed in integration-active-view.tsx (the
objects created when secretBindings/SENSITIVE_KEYS, oauth2 auth branch using
hasOAuth2Config, and the integration.connectionConfig?.domain and ?.apiEndpoint
branches) to call t(...) with appropriate keys (e.g.
integrations.manageDialog.apiKey, integrations.manageDialog.accessToken,
integrations.manageDialog.domain, integrations.manageDialog.apiEndpoint) so the
UI shows translated labels consistently.
---
Outside diff comments:
In `@services/platform/app/components/icons/gmail-icon.tsx`:
- Around line 1-11: GmailIcon is missing an accessibility attribute—match other
icons (like OutlookIcon) by adding either an aria-label when the icon is
meaningful or aria-hidden="true" when decorative; update the GmailIcon JSX root
<svg> to include the chosen attribute (or accept and forward an ariaLabel prop)
so accessibility usage is consistent with the other icon components and the cn
utility usage remains unchanged.
In `@services/platform/app/components/icons/outlook-icon.tsx`:
- Around line 1-9: The OutlookIcon component's <svg> (in the OutlookIcon
function) is missing an accessibility attribute for consistency; update the SVG
element inside OutlookIcon to include either aria-label="Outlook icon" (if it
conveys meaning) or aria-hidden="true" (if decorative) while keeping the
existing className usage via cn and props signature unchanged so other icons
remain consistent.
In `@services/platform/app/components/ui/layout/separator.stories.tsx`:
- Around line 48-59: The Horizontal story now uses args to control Separator
props; update the other stories (Vertical and InToolbar) to the same args-based
pattern by moving the hardcoded orientation="vertical" into their args objects
and using render: (args) => <Separator {...args} /> (or wrap in the same
container JSX used by those stories), so both Vertical and InToolbar accept
dynamic props via args while still rendering the Separator component.
In `@services/platform/app/features/websites/components/website-view-dialog.tsx`:
- Around line 104-120: The StatGrid items currently contain hardcoded label keys
('domain' and 'apiEndpoint') in the items array inside website-view-dialog.tsx;
replace those literal strings with calls to the app's i18n helper (e.g.,
useTranslation/useIntl and t(...) or formatMessage(...)) so the labels are
localized before being passed into StatGrid, updating the items construction in
the WebsiteViewDialog component to use translated values for the 'domain' and
'apiEndpoint' labels.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (66)
services/platform/app/components/icons/circuly-icon.tsxservices/platform/app/components/icons/enter-key-icon.tsxservices/platform/app/components/icons/gmail-icon.tsxservices/platform/app/components/icons/locale-icon.tsxservices/platform/app/components/icons/microsoft-icon.tsxservices/platform/app/components/icons/onedrive-icon.tsxservices/platform/app/components/icons/outlook-icon.tsxservices/platform/app/components/icons/protel-icon.tsxservices/platform/app/components/icons/sharepoint-icon.tsxservices/platform/app/components/icons/shopify-icon.tsxservices/platform/app/components/icons/website-icon.tsxservices/platform/app/components/ui/data-display/file-preview-card.tsxservices/platform/app/components/ui/data-display/inline-code.stories.tsxservices/platform/app/components/ui/data-display/selectable-row.stories.tsxservices/platform/app/components/ui/data-display/stat-grid.stories.tsxservices/platform/app/components/ui/data-display/stat-grid.test.tsxservices/platform/app/components/ui/data-display/stat-grid.tsxservices/platform/app/components/ui/data-display/stat-item.stories.tsxservices/platform/app/components/ui/data-display/stat-item.test.tsxservices/platform/app/components/ui/data-display/stat-item.tsxservices/platform/app/components/ui/data-display/table-date-cell.stories.tsxservices/platform/app/components/ui/dialog/form-dialog.stories.tsxservices/platform/app/components/ui/feedback/alert.stories.tsxservices/platform/app/components/ui/feedback/badge.stories.tsxservices/platform/app/components/ui/feedback/progress.stories.tsxservices/platform/app/components/ui/feedback/spinner.stories.tsxservices/platform/app/components/ui/feedback/validation-check-item.stories.tsxservices/platform/app/components/ui/forms/checkbox-group.stories.tsxservices/platform/app/components/ui/forms/checkbox.tsxservices/platform/app/components/ui/forms/description.stories.tsxservices/platform/app/components/ui/forms/form-section.stories.tsxservices/platform/app/components/ui/forms/radio-group.stories.tsxservices/platform/app/components/ui/forms/search-input.stories.tsxservices/platform/app/components/ui/layout/action-row.stories.tsxservices/platform/app/components/ui/layout/bordered-section.stories.tsxservices/platform/app/components/ui/layout/card.stories.tsxservices/platform/app/components/ui/layout/page-section.stories.tsxservices/platform/app/components/ui/layout/separator.stories.tsxservices/platform/app/components/ui/logo/logo.stories.tsxservices/platform/app/components/ui/navigation/collapsible-details.stories.tsxservices/platform/app/components/ui/navigation/tab-navigation.stories.tsxservices/platform/app/components/ui/navigation/tabs.stories.tsxservices/platform/app/components/ui/overlays/dropdown-menu.stories.tsxservices/platform/app/components/ui/overlays/popover.stories.tsxservices/platform/app/components/ui/overlays/tooltip.stories.tsxservices/platform/app/components/ui/primitives/button.stories.tsxservices/platform/app/components/ui/primitives/icon-button.stories.tsxservices/platform/app/components/ui/typography/heading.stories.tsxservices/platform/app/components/ui/typography/text.stories.tsxservices/platform/app/features/approvals/components/approval-detail-dialog.tsxservices/platform/app/features/chat/components/chat-input.tsxservices/platform/app/features/chat/components/message-info-dialog.tsxservices/platform/app/features/chat/components/sub-agent-details-dialog.tsxservices/platform/app/features/customers/components/customer-information.tsxservices/platform/app/features/products/components/product-view-dialog.tsxservices/platform/app/features/settings/integrations/components/integration-details.tsxservices/platform/app/features/settings/integrations/components/integration-manage/integration-active-view.tsxservices/platform/app/features/settings/integrations/components/sso-config-dialog.tsxservices/platform/app/features/vendors/components/vendor-information.tsxservices/platform/app/features/websites/components/website-view-dialog.tsxservices/platform/app/routes/_auth/log-in.tsxservices/platform/app/routes/_auth/sign-up.tsxservices/platform/app/routes/dashboard/$id/_knowledge.tsxservices/platform/app/routes/dashboard/$id/approvals.tsxservices/platform/app/routes/dashboard/$id/settings.tsxservices/platform/tsr.config.json
| export const OneDriveIcon = ({ className }: { className?: string }) => { | ||
| return ( | ||
| <svg viewBox="0 0 31 20" className={className} aria-hidden> | ||
| <svg viewBox="0 0 31 20" className={cn('size-full', className)} aria-hidden> |
There was a problem hiding this comment.
Fix Biome a11y lint failure on SVG title.
Line 5 currently triggers lint/a11y/noSvgWithoutTitle. Add a non-empty <title> (or adjust accessibility semantics) to unblock lint.
Proposed fix
- <svg viewBox="0 0 31 20" className={cn('size-full', className)} aria-hidden>
+ <svg
+ viewBox="0 0 31 20"
+ className={cn('size-full', className)}
+ aria-hidden
+ >
+ <title>OneDrive</title>📝 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.
| <svg viewBox="0 0 31 20" className={cn('size-full', className)} aria-hidden> | |
| <svg | |
| viewBox="0 0 31 20" | |
| className={cn('size-full', className)} | |
| aria-hidden | |
| > | |
| <title>OneDrive</title> |
🧰 Tools
🪛 Biome (2.4.4)
[error] 5-5: Alternative text title element cannot be empty
(lint/a11y/noSvgWithoutTitle)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/app/components/icons/onedrive-icon.tsx` at line 5, Add a
non-empty <title> element for the SVG in the OnedriveIcon component to satisfy
the lint rule; insert a unique id (e.g., titleId) and render <title
id={titleId}>OneDrive</title> inside the <svg>, then add
aria-labelledby={titleId} (or role="img") on the same <svg> element instead of
aria-hidden so screen readers can read the title; update the component props to
generate/accept titleId if needed and keep existing className/cn usage intact.
| args: { | ||
| selected: false, | ||
| children: <Text>Unselected row</Text>, | ||
| }, |
There was a problem hiding this comment.
Use inline text tags inside SelectableRow button content.
Text defaults to a <p> tag, so these stories currently render a block <p> inside a <button>. Please switch to as="span" (or another phrasing element) for valid semantics.
Suggested fix
export const Default: Story = {
args: {
selected: false,
- children: <Text>Unselected row</Text>,
+ children: <Text as="span">Unselected row</Text>,
},
};
export const Selected: Story = {
args: {
selected: true,
- children: <Text>Selected row</Text>,
+ children: <Text as="span">Selected row</Text>,
},Also applies to: 59-62
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/app/components/ui/data-display/selectable-row.stories.tsx`
around lines 52 - 55, The stories render a block-level <Text> inside the
SelectableRow button causing invalid semantics; update the story args so the
children use an inline element (e.g., change the Text usage to Text with
as="span" or another phrasing element) for both the unselected and selected
story entries (look for the SelectableRow story args where children is currently
<Text>Unselected row</Text> and the other case at the following story with
children <Text>Selected row</Text> and replace them accordingly).
| export const WithColSpan: Story = { | ||
| args: { | ||
| items: [ | ||
| { label: 'Domain', value: <Text>example.com</Text> }, | ||
| { | ||
| label: 'Status', | ||
| value: ( | ||
| <Badge variant="green" dot> | ||
| Active | ||
| </Badge> | ||
| ), | ||
| }, | ||
| { | ||
| label: 'Description', | ||
| value: ( | ||
| <Text> | ||
| A full-width stat item that spans both columns in the grid. | ||
| </Text> | ||
| ), | ||
| colSpan: 2, | ||
| }, | ||
| ], | ||
| }, | ||
| parameters: { | ||
| docs: { | ||
| description: { | ||
| story: | ||
| 'Use `colSpan: 2` on an item to span the full width of a 2-column grid.', | ||
| }, | ||
| }, | ||
| }, | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider clarifying the default column count in the colSpan example.
The story description mentions "full width of a 2-column grid," but the story doesn't explicitly set cols. If 2 columns is the default, this works correctly. However, making it explicit would improve self-documentation.
📝 Optional: Make the column count explicit
export const WithColSpan: Story = {
args: {
+ cols: 2,
items: [
{ label: 'Domain', value: <Text>example.com</Text> },📝 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.
| export const WithColSpan: Story = { | |
| args: { | |
| items: [ | |
| { label: 'Domain', value: <Text>example.com</Text> }, | |
| { | |
| label: 'Status', | |
| value: ( | |
| <Badge variant="green" dot> | |
| Active | |
| </Badge> | |
| ), | |
| }, | |
| { | |
| label: 'Description', | |
| value: ( | |
| <Text> | |
| A full-width stat item that spans both columns in the grid. | |
| </Text> | |
| ), | |
| colSpan: 2, | |
| }, | |
| ], | |
| }, | |
| parameters: { | |
| docs: { | |
| description: { | |
| story: | |
| 'Use `colSpan: 2` on an item to span the full width of a 2-column grid.', | |
| }, | |
| }, | |
| }, | |
| }; | |
| export const WithColSpan: Story = { | |
| args: { | |
| cols: 2, | |
| items: [ | |
| { label: 'Domain', value: <Text>example.com</Text> }, | |
| { | |
| label: 'Status', | |
| value: ( | |
| <Badge variant="green" dot> | |
| Active | |
| </Badge> | |
| ), | |
| }, | |
| { | |
| label: 'Description', | |
| value: ( | |
| <Text> | |
| A full-width stat item that spans both columns in the grid. | |
| </Text> | |
| ), | |
| colSpan: 2, | |
| }, | |
| ], | |
| }, | |
| parameters: { | |
| docs: { | |
| description: { | |
| story: | |
| 'Use `colSpan: 2` on an item to span the full width of a 2-column grid.', | |
| }, | |
| }, | |
| }, | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/app/components/ui/data-display/stat-grid.stories.tsx`
around lines 90 - 121, The story "WithColSpan" currently relies on an implicit
2-column default but doesn't declare it; update the Story object (WithColSpan)
to explicitly set the grid column count so the colSpan: 2 example is
self-documenting — add a cols: 2 (or the appropriate prop name your StatGrid
expects) to the args alongside items so the example clearly shows the full-width
behavior for a 2-column grid.
| {items.map((item) => ( | ||
| <StatItem key={item.label} label={item.label} colSpan={item.colSpan}> | ||
| {item.value} | ||
| </StatItem> | ||
| ))} |
There was a problem hiding this comment.
Potential key collision with duplicate labels.
Using item.label as the React key could cause issues if the same label appears multiple times in the items array. Consider adding an optional id field to StatGridItem or using the array index as a fallback.
🔑 Proposed fix to handle duplicate labels
Option 1: Use index as fallback key:
{items.map((item, index) => (
- <StatItem key={item.label} label={item.label} colSpan={item.colSpan}>
+ <StatItem key={`${item.label}-${index}`} label={item.label} colSpan={item.colSpan}>
{item.value}
</StatItem>
))}Option 2: Add optional id to interface:
export interface StatGridItem {
+ id?: string;
label: string;
value: ReactNode;
colSpan?: 1 | 2;
}Then use:
- <StatItem key={item.label} label={item.label} colSpan={item.colSpan}>
+ <StatItem key={item.id ?? item.label} label={item.label} colSpan={item.colSpan}>📝 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.
| {items.map((item) => ( | |
| <StatItem key={item.label} label={item.label} colSpan={item.colSpan}> | |
| {item.value} | |
| </StatItem> | |
| ))} | |
| {items.map((item, index) => ( | |
| <StatItem key={`${item.label}-${index}`} label={item.label} colSpan={item.colSpan}> | |
| {item.value} | |
| </StatItem> | |
| ))} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/app/components/ui/data-display/stat-grid.tsx` around lines
42 - 46, The current map uses item.label as the React key which can collide;
update the StatGridItem type to include an optional unique id (e.g., id?:
string) and change the mapping in StatGrid component to use a stable key
fallback like item.id ?? item.label ?? index (use the array index only as last
resort). Ensure StatItem rendering still receives label and colSpan props and
keep the key logic inside the items.map callback so duplicate labels no longer
cause key collisions.
| export const UploadExample: Story = { | ||
| render: () => ( | ||
| args: { | ||
| value: 67, | ||
| label: 'Uploading document.pdf', | ||
| }, | ||
| render: (args) => ( | ||
| <div className="flex flex-col gap-2"> | ||
| <div className="flex justify-between text-sm"> | ||
| <span>Uploading document.pdf</span> | ||
| <span>67%</span> | ||
| </div> | ||
| <Progress value={67} label="Uploading document.pdf" /> | ||
| <Progress {...args} /> | ||
| </div> | ||
| ), |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Hardcoded text won't sync with Storybook controls.
The displayed text ("Uploading document.pdf" on line 158 and "67%" on line 159) is hardcoded, but the args can be changed via Storybook controls. This means adjusting value or label in the controls panel will update the progress bar but not the surrounding text.
♻️ Suggested fix to derive text from args
export const UploadExample: Story = {
args: {
value: 67,
label: 'Uploading document.pdf',
},
render: (args) => (
<div className="flex flex-col gap-2">
<div className="flex justify-between text-sm">
- <span>Uploading document.pdf</span>
- <span>67%</span>
+ <span>{args.label}</span>
+ <span>{args.value}%</span>
</div>
<Progress {...args} />
</div>
),
};📝 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.
| export const UploadExample: Story = { | |
| render: () => ( | |
| args: { | |
| value: 67, | |
| label: 'Uploading document.pdf', | |
| }, | |
| render: (args) => ( | |
| <div className="flex flex-col gap-2"> | |
| <div className="flex justify-between text-sm"> | |
| <span>Uploading document.pdf</span> | |
| <span>67%</span> | |
| </div> | |
| <Progress value={67} label="Uploading document.pdf" /> | |
| <Progress {...args} /> | |
| </div> | |
| ), | |
| export const UploadExample: Story = { | |
| args: { | |
| value: 67, | |
| label: 'Uploading document.pdf', | |
| }, | |
| render: (args) => ( | |
| <div className="flex flex-col gap-2"> | |
| <div className="flex justify-between text-sm"> | |
| <span>{args.label}</span> | |
| <span>{args.value}%</span> | |
| </div> | |
| <Progress {...args} /> | |
| </div> | |
| ), | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/app/components/ui/feedback/progress.stories.tsx` around
lines 150 - 163, The story hardcodes the label and percentage so Storybook
controls don't update the surrounding text; update UploadExample's render to
read from the story args (use args.label and args.value) and render the label
and percentage dynamically alongside <Progress {...args} /> so changing
args.value or args.label in controls updates both the bar and the displayed
text.
| {(() => { | ||
| const tokenItems: StatGridItem[] = [ | ||
| ...(metadata.contextWindow | ||
| ? [ | ||
| { | ||
| label: t('messageInfo.contextWindow'), | ||
| value: ( | ||
| <ContextWindowToken | ||
| contextWindow={metadata.contextWindow} | ||
| contextStats={metadata.contextStats} | ||
| t={t} | ||
| tCommon={tCommon} | ||
| locale={locale} | ||
| /> | ||
| ), | ||
| }, | ||
| ] | ||
| : []), | ||
| ...( | ||
| [ | ||
| [metadata.inputTokens, 'input'], | ||
| [metadata.outputTokens, 'output'], | ||
| [metadata.totalTokens, 'total'], | ||
| [metadata.reasoningTokens, 'reasoning'], | ||
| [metadata.cachedInputTokens, 'cached'], | ||
| ] as [number | undefined, string][] | ||
| ) | ||
| .filter( | ||
| (entry): entry is [number, string] => | ||
| entry[0] != null && entry[0] > 0, | ||
| ) | ||
| .map(([value, key]) => ({ | ||
| label: t(`messageInfo.${key}`), | ||
| value: <Text>{formatNumber(value, locale)}</Text>, | ||
| })), | ||
| ]; | ||
| return tokenItems.length > 0 ? ( | ||
| <Field label={t('messageInfo.tokenUsage')}> | ||
| <StatGrid className="text-sm" items={tokenItems} /> | ||
| </Field> | ||
| ) : null; | ||
| })()} |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider extracting IIFE logic to improve readability.
The inline IIFE pattern (() => { ... })() works but adds visual complexity. Consider extracting the tokenItems computation to a useMemo or a variable declared before the return statement for better readability and potential memoization benefits.
♻️ Suggested refactor to extract tokenItems
Move the token items computation before the return statement:
const tokenItems: StatGridItem[] = useMemo(() => [
...(metadata?.contextWindow
? [{
label: t('messageInfo.contextWindow'),
value: (
<ContextWindowToken
contextWindow={metadata.contextWindow}
contextStats={metadata.contextStats}
t={t}
tCommon={tCommon}
locale={locale}
/>
),
}]
: []),
...([
[metadata?.inputTokens, 'input'],
[metadata?.outputTokens, 'output'],
[metadata?.totalTokens, 'total'],
[metadata?.reasoningTokens, 'reasoning'],
[metadata?.cachedInputTokens, 'cached'],
] as [number | undefined, string][])
.filter((entry): entry is [number, string] => entry[0] != null && entry[0] > 0)
.map(([value, key]) => ({
label: t(`messageInfo.${key}`),
value: <Text>{formatNumber(value, locale)}</Text>,
})),
], [metadata, t, tCommon, locale]);Then in the JSX:
{tokenItems.length > 0 && (
<Field label={t('messageInfo.tokenUsage')}>
<StatGrid className="text-sm" items={tokenItems} />
</Field>
)}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/app/features/chat/components/message-info-dialog.tsx`
around lines 157 - 198, Extract the IIFE that builds tokenItems into a top-level
constant (preferably using useMemo) to improve readability and memoize
calculations: create a const tokenItems = useMemo(() => { ... }, [metadata, t,
tCommon, locale]) that constructs the same array (including the conditional
ContextWindowToken entry and the filtered/mapped token tuples) and then replace
the inline IIFE in the JSX with a simple conditional render like
tokenItems.length > 0 && (<Field ...><StatGrid ... items={tokenItems}
/></Field>). Ensure you reference the same symbols (metadata,
ContextWindowToken, StatGrid, Field, t, tCommon, locale, formatNumber) and keep
the same filtering predicate ([number | undefined, string] => entry[0] != null
&& entry[0] > 0) so behavior is unchanged.
| {(() => { | ||
| const perfItems: StatGridItem[] = ( | ||
| [ | ||
| [metadata.durationMs, 'duration'], | ||
| [metadata.timeToFirstTokenMs, 'timeToFirstToken'], | ||
| ] as [number | undefined, string][] | ||
| ) | ||
| .filter((entry): entry is [number, string] => entry[0] != null) | ||
| .map(([value, key]) => ({ | ||
| label: t(`messageInfo.${key}`), | ||
| value: <Text>{(value / 1000).toFixed(2)}s</Text>, | ||
| })); | ||
| return perfItems.length > 0 ? ( | ||
| <Field label={t('messageInfo.performance')}> | ||
| <StatGrid className="text-sm" items={perfItems} /> | ||
| </Field> | ||
| ) : null; | ||
| })()} |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Same IIFE pattern observation applies here.
The performance items IIFE follows the same pattern. Consider extracting to a useMemo alongside the token items for consistency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/app/features/chat/components/message-info-dialog.tsx`
around lines 200 - 217, The IIFE that builds perfItems in
message-info-dialog.tsx should be extracted into a memoized value using useMemo
to match the token items pattern; replace the IIFE with a const perfItems =
useMemo(() => { /* build array from metadata.durationMs and
metadata.timeToFirstTokenMs, filter out null/undefined, map to {label:
t(`messageInfo.${key}`), value: <Text>{(value/1000).toFixed(2)}s</Text>} */ },
[metadata.durationMs, metadata.timeToFirstTokenMs, t]) and render the
Field/StatGrid conditionally based on perfItems.length > 0 as before.
| label: t('labels.locale'), | ||
| value: <Text>{customer.locale || 'en'}</Text>, | ||
| }, |
There was a problem hiding this comment.
Don’t default missing customer locale to en in display.
Line 50 can misstate unknown data as English. Prefer showing notAvailable (or a true system default locale source) instead of a hardcoded fallback.
💡 Suggested patch
- value: <Text>{customer.locale || 'en'}</Text>,
+ value: <Text>{customer.locale || t('labels.notAvailable')}</Text>,📝 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.
| label: t('labels.locale'), | |
| value: <Text>{customer.locale || 'en'}</Text>, | |
| }, | |
| label: t('labels.locale'), | |
| value: <Text>{customer.locale || t('labels.notAvailable')}</Text>, | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/app/features/customers/components/customer-information.tsx`
around lines 49 - 51, The UI currently hardcodes a fallback of 'en' when
customer.locale is missing in the customer-information component; update the
locale cell (the object with label: t('labels.locale') and value) to avoid
showing 'en' as a presumed locale and instead render a proper "not available"
indicator or the true system default: replace the hardcoded string usage of 'en'
with a translation placeholder (e.g., t('notAvailable')) or a conditional that
displays t('notAvailable') when customer.locale is falsy (or use the configured
system default source if available), updating the value expression where
customer.locale is referenced.
| ...(integration.sqlConnectionConfig.readOnly != null | ||
| ? [ | ||
| { | ||
| label: t('integrations.upload.readOnly'), | ||
| value: integration.sqlConnectionConfig.readOnly | ||
| ? 'Yes' | ||
| : 'No', | ||
| }, | ||
| ] | ||
| : []), |
There was a problem hiding this comment.
Internationalize "Yes"/"No" strings.
The readOnly value renders hardcoded "Yes"/"No" strings. These should use the i18n translation function for consistency with the rest of the UI.
🌐 Proposed fix for i18n
{
label: t('integrations.upload.readOnly'),
- value: integration.sqlConnectionConfig.readOnly
- ? 'Yes'
- : 'No',
+ value: integration.sqlConnectionConfig.readOnly
+ ? t('common.yes')
+ : t('common.no'),
},Note: This assumes t('common.yes') and t('common.no') exist in the common namespace. Verify the correct translation keys are available.
📝 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.
| ...(integration.sqlConnectionConfig.readOnly != null | |
| ? [ | |
| { | |
| label: t('integrations.upload.readOnly'), | |
| value: integration.sqlConnectionConfig.readOnly | |
| ? 'Yes' | |
| : 'No', | |
| }, | |
| ] | |
| : []), | |
| ...(integration.sqlConnectionConfig.readOnly != null | |
| ? [ | |
| { | |
| label: t('integrations.upload.readOnly'), | |
| value: integration.sqlConnectionConfig.readOnly | |
| ? t('common.yes') | |
| : t('common.no'), | |
| }, | |
| ] | |
| : []), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@services/platform/app/features/settings/integrations/components/integration-details.tsx`
around lines 348 - 357, The displayed readOnly strings are hardcoded "Yes"/"No";
replace them with translated values by calling the i18n function (e.g., use
t('common.yes') and t('common.no')) where the value is set for
integration.sqlConnectionConfig.readOnly inside the integration-details.tsx
component so the label value becomes integration.sqlConnectionConfig.readOnly ?
t('common.yes') : t('common.no'); ensure you import/use the existing t function
already used in this file.
| label: | ||
| secretBindings.find((b) => SENSITIVE_KEYS.has(b)) ?? 'apiKey', | ||
| value: <Text variant="code">{'\u00d7'.repeat(8)}</Text>, | ||
| }, | ||
| ] | ||
| : []), | ||
| ...(integration.authMethod === 'oauth2' && integration.oauth2Auth | ||
| ? [ | ||
| { | ||
| label: hasOAuth2Config | ||
| ? t('integrations.manageDialog.connectedViaOAuth2') | ||
| : 'accessToken', | ||
| value: <Text variant="code">{'\u00d7'.repeat(8)}</Text>, | ||
| }, | ||
| ] | ||
| : []), | ||
| ...(integration.connectionConfig?.domain | ||
| ? [ | ||
| { | ||
| label: 'domain', | ||
| value: ( | ||
| <Text variant="code" truncate> | ||
| {maskValue(integration.connectionConfig.domain)} | ||
| </Text> | ||
| ), | ||
| }, | ||
| ] | ||
| : []), | ||
| ...(integration.connectionConfig?.apiEndpoint | ||
| ? [ | ||
| { | ||
| label: 'apiEndpoint', | ||
| value: ( |
There was a problem hiding this comment.
Avoid raw/internal auth labels in UI.
Several labels are hardcoded/internal ('apiKey', 'accessToken', 'domain', 'apiEndpoint'). These should be translated/user-facing for consistent localized settings UI.
💡 Suggested direction
- secretBindings.find((b) => SENSITIVE_KEYS.has(b)) ?? 'apiKey',
+ secretBindings.find((b) => SENSITIVE_KEYS.has(b)) ??
+ t('integrations.manageDialog.apiKey'),
@@
- : 'accessToken',
+ : t('integrations.manageDialog.accessToken'),
@@
- label: 'domain',
+ label: t('integrations.manageDialog.domain'),
@@
- label: 'apiEndpoint',
+ label: t('integrations.manageDialog.apiEndpoint'),📝 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.
| label: | |
| secretBindings.find((b) => SENSITIVE_KEYS.has(b)) ?? 'apiKey', | |
| value: <Text variant="code">{'\u00d7'.repeat(8)}</Text>, | |
| }, | |
| ] | |
| : []), | |
| ...(integration.authMethod === 'oauth2' && integration.oauth2Auth | |
| ? [ | |
| { | |
| label: hasOAuth2Config | |
| ? t('integrations.manageDialog.connectedViaOAuth2') | |
| : 'accessToken', | |
| value: <Text variant="code">{'\u00d7'.repeat(8)}</Text>, | |
| }, | |
| ] | |
| : []), | |
| ...(integration.connectionConfig?.domain | |
| ? [ | |
| { | |
| label: 'domain', | |
| value: ( | |
| <Text variant="code" truncate> | |
| {maskValue(integration.connectionConfig.domain)} | |
| </Text> | |
| ), | |
| }, | |
| ] | |
| : []), | |
| ...(integration.connectionConfig?.apiEndpoint | |
| ? [ | |
| { | |
| label: 'apiEndpoint', | |
| value: ( | |
| label: | |
| secretBindings.find((b) => SENSITIVE_KEYS.has(b)) ?? | |
| t('integrations.manageDialog.apiKey'), | |
| value: <Text variant="code">{'\u00d7'.repeat(8)}</Text>, | |
| }, | |
| ] | |
| : []), | |
| ...(integration.authMethod === 'oauth2' && integration.oauth2Auth | |
| ? [ | |
| { | |
| label: hasOAuth2Config | |
| ? t('integrations.manageDialog.connectedViaOAuth2') | |
| : t('integrations.manageDialog.accessToken'), | |
| value: <Text variant="code">{'\u00d7'.repeat(8)}</Text>, | |
| }, | |
| ] | |
| : []), | |
| ...(integration.connectionConfig?.domain | |
| ? [ | |
| { | |
| label: t('integrations.manageDialog.domain'), | |
| value: ( | |
| <Text variant="code" truncate> | |
| {maskValue(integration.connectionConfig.domain)} | |
| </Text> | |
| ), | |
| }, | |
| ] | |
| : []), | |
| ...(integration.connectionConfig?.apiEndpoint | |
| ? [ | |
| { | |
| label: t('integrations.manageDialog.apiEndpoint'), | |
| value: ( |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@services/platform/app/features/settings/integrations/components/integration-manage/integration-active-view.tsx`
around lines 88 - 120, Replace the hardcoded/internal labels ('apiKey',
'accessToken', 'domain', 'apiEndpoint') used when building the displayed items
with user-facing, localized strings via the translation function (t). Update the
label entries in the arrays constructed in integration-active-view.tsx (the
objects created when secretBindings/SENSITIVE_KEYS, oauth2 auth branch using
hasOAuth2Config, and the integration.connectionConfig?.domain and ?.apiEndpoint
branches) to call t(...) with appropriate keys (e.g.
integrations.manageDialog.apiKey, integrations.manageDialog.accessToken,
integrations.manageDialog.domain, integrations.manageDialog.apiEndpoint) so the
UI shows translated labels consistently.
Introduce StatGrid for structured data display using definition lists, refactor icon components to use responsive sizing with cn(), improve checkbox layout with description alignment, refactor detail dialogs to use StatGrid, and update Storybook stories across UI components.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes & Improvements