feat(platform): polish settings logs page and ID-field component#1649
Conversation
Restructures the settings logs page with a dedicated heading, hoists category filtering and CSV/JSON exports to the page header, and refreshes the sign-in blocks table with new columns (timestamp, user, IP, lock-out, IP limit). Audit log timestamps now use an absolute MMM D, YYYY HH:mm format instead of relative time. Redesigns CopyableField as a single bordered "ID field" pill matching the design-system pattern, adopts it for the organization ID with a helper subtitle, and removes hardcoded heights from button/search-input/ filter-button so size variants stay consistent.
📝 WalkthroughWalkthroughThis pull request refactors multiple UI components and audit log functionality across the platform. Key changes include restructuring Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Review rate limit: 4/5 reviews remaining, refill in 12 minutes. 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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
services/platform/app/routes/dashboard/$id/settings/logs.tsx (1)
148-184:⚠️ Potential issue | 🟠 MajorHide the audit-only controls when another tab is selected.
The category filter and CSV/JSON actions now render outside the audit tab, but they still operate only on audit-log state (
search.categoryandrequestExport). On the “Sign-in blocks” tab, users can therefore export/filter a different dataset than the one currently on screen. Please gate these controls behind the active tab, or move them back into audit-specific UI.Also applies to: 186-230
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform/app/routes/dashboard/`$id/settings/logs.tsx around lines 148 - 184, The export buttons and category filter must be rendered only when the audit tab is active because they operate on audit-only state (search.category and requestExport); update the conditional that currently uses isAdminUser to also check the active tab (e.g., activeTab === 'audit' or selectedTab === 'audit') around the block that renders the Button components and the category filter, or move those controls into the audit-tab panel so handleExport and exportAction only affect the visible audit data; apply the same gating fix for the similar controls referenced around lines 186-230.
🤖 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/ui/data-display/copyable-field.tsx`:
- Around line 49-51: The Label currently uses htmlFor={valueId} and the copy
Button relies on aria-label which causes the label to bind to the button and the
accessible name to be just "Copy"; change this so the Label is not bound to the
button (remove htmlFor on Label or point it only at the actual input if present)
and set the copy Button to use aria-labelledby referencing the label and value
IDs (valueId and the label's id derived from reactId) plus statusId as needed so
the button's accessible name is composed of label + value (and status) rather
than overridden by aria-label; update references to reactId, valueId, statusId,
Label and the copy Button and remove the label-click-to-copy behavior.
In
`@services/platform/app/features/settings/audit-logs/components/block-counters-table.tsx`:
- Around line 64-67: Replace the hardcoded fallback '—' in the cell renderer
with the translated empty-cell placeholder: import and use the translation hook
(e.g., useTranslation / t) in the component containing the cell renderer (the
block-counters-table component) and change the expression returning
{row.original.lastIp ?? '—'} to use t('tables.cells.empty') (or the equivalent
key) so the Text component renders the localized placeholder when lastIp is
null/undefined.
In
`@services/platform/app/features/settings/organization/components/organization-settings.tsx`:
- Line 218: The CopyableField for organization._id currently uses a generic aria
label; update the JSX to pass a context-specific copyAriaLabel (e.g., "Copy
organization ID") using your app's i18n helper (for example call
t('settings.copyOrganizationId') or useTranslation() to build the string) so
screen readers can distinguish this control; modify the organization-settings
component where <CopyableField value={organization._id} /> is rendered to
include copyAriaLabel={t('...')} (and import/use the translation hook if not
already present).
In `@services/platform/messages/en.json`:
- Around line 1160-1164: The JSON translation values for the keys "ipAddress",
"lockOut", and "ipLimit" are in title case; change them to sentence case to
match repository translation style by updating the values to "IP address", "Lock
out", and "IP limit" respectively (leave "timestamp" and "user" as-is), ensuring
the keys "ipAddress", "lockOut", and "ipLimit" remain unchanged so downstream
locales inherit the corrected baseline.
In `@services/platform/messages/fr.json`:
- Line 1146: Replace the formal French imperative in the "subheading" string
with the informal "tu" form; locate the JSON key "subheading" (currently
"Consultez et gérez les journaux système.") and change its value to the informal
imperative version such as "Consulte et gère les journaux système." so it
follows the project guideline to use informal French across logs.
---
Outside diff comments:
In `@services/platform/app/routes/dashboard/`$id/settings/logs.tsx:
- Around line 148-184: The export buttons and category filter must be rendered
only when the audit tab is active because they operate on audit-only state
(search.category and requestExport); update the conditional that currently uses
isAdminUser to also check the active tab (e.g., activeTab === 'audit' or
selectedTab === 'audit') around the block that renders the Button components and
the category filter, or move those controls into the audit-tab panel so
handleExport and exportAction only affect the visible audit data; apply the same
gating fix for the similar controls referenced around lines 186-230.
🪄 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: 4254c863-a974-43c5-b3e1-dc361b8afe19
📒 Files selected for processing (16)
services/platform/app/components/ui/data-display/copyable-field.test.tsxservices/platform/app/components/ui/data-display/copyable-field.tsxservices/platform/app/components/ui/data-display/table-date-cell.tsxservices/platform/app/components/ui/filters/filter-button.tsxservices/platform/app/components/ui/forms/search-input.tsxservices/platform/app/components/ui/navigation/tabs.tsxservices/platform/app/components/ui/primitives/button.tsxservices/platform/app/features/settings/audit-logs/components/audit-log-table.tsxservices/platform/app/features/settings/audit-logs/components/block-counters-table.tsxservices/platform/app/features/settings/audit-logs/hooks/use-audit-log-table-config.tsxservices/platform/app/features/settings/organization/components/organization-settings.tsxservices/platform/app/features/settings/providers/components/provider-add-panel.tsxservices/platform/app/routes/dashboard/$id/settings/logs.tsxservices/platform/messages/de.jsonservices/platform/messages/en.jsonservices/platform/messages/fr.json
💤 Files with no reviewable changes (1)
- services/platform/app/features/settings/providers/components/provider-add-panel.tsx
| const reactId = React.useId(); | ||
| const valueId = `${reactId}-value`; | ||
| const statusId = `${reactId}-status`; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For native HTML accessibility: if a <label for="..."> points at a <button>, does clicking the label activate the button, and does aria-label override both the visible button text and associated <label> when computing the accessible name?
💡 Result:
Yes, clicking a that points to a activates (triggers the click on) the button. Yes, aria-label on the button overrides both the button's visible text content and any associated text when computing the accessible name, per the accessible name computation algorithm priority (aria-label after aria-labelledby but before contents and host language labels). The accessible name for a button is computed by the algorithm in Accessible Name and Description Computation 1.2 (W3C): first aria-labelledby (if present), then aria-label (overrides everything below), then host language (including associated label(s)), then name from content (button text). For activation: The HTML spec defines as labelable, so associates with it (button.labels includes the label). Clicking the label fires a click event on the button (focus + synthetic click), activating it. MDN confirms labelable elements like button support this, though primarily documented for inputs; spec confirms button is labelable.
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/Label
- 2: https://www.w3.org/WAI/ARIA/apg/practices/names-and-descriptions/
- 3: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Reference/Attributes/aria-label
- 4: https://www.w3.org/TR/2026/WD-accname-1.2-20260311/
- 5: https://html.spec.whatwg.org/multipage/the-button-element.html
- 6: https://www.w3.org/TR/accname-1.2
- 7: https://developer.mozilla.org/en-US/docs/Web/API/HTMLButtonElement/labels
- 8: http://www.w3.org/TR/html5/the-label-element.html
- 9: https://html.spec.whatwg.org/multipage/forms.html
🏁 Script executed:
cat -n services/platform/app/components/ui/data-display/copyable-field.tsx | head -90Repository: tale-project/tale
Length of output: 3521
🏁 Script executed:
# Check if copyAriaLabel is typically provided in usages of CopyableField
rg -A 3 "CopyableField" --type tsx | head -50Repository: tale-project/tale
Length of output: 89
🏁 Script executed:
# Search for CopyableField usages across the codebase
rg "CopyableField" -A 3 -t ts -t js | head -80Repository: tale-project/tale
Length of output: 7446
🏁 Script executed:
# Check the test file to understand accessibility expectations
cat -n services/platform/app/components/ui/data-display/copyable-field.test.tsx | head -60Repository: tale-project/tale
Length of output: 1969
🏁 Script executed:
# Check the stories to see how copyAriaLabel is used in practice
cat -n services/platform/app/components/ui/data-display/copyable-field.stories.tsx | grep -A 10 -B 10 "copyAriaLabel"Repository: tale-project/tale
Length of output: 43
🏁 Script executed:
# Search for any actual usage of copyAriaLabel parameter
rg "copyAriaLabel" --type ts --type jsRepository: tale-project/tale
Length of output: 626
🏁 Script executed:
# Check if the actual accessible name includes the label context
# Look at the full test and see what the actual behavior might be
cat -n services/platform/app/components/ui/data-display/copyable-field.stories.tsx | head -120Repository: tale-project/tale
Length of output: 3750
🏁 Script executed:
# Check what checkAccessibility (axe) actually validates
rg -A 10 "checkAccessibility" --type ts --type js | head -30Repository: tale-project/tale
Length of output: 2929
🏁 Script executed:
# Let's trace through what the actual accessible name computation would be
# First, check if there are any other accessibility tests or comments about this
rg -B 5 -A 5 "aria-label.*Copy" services/platform/app/components/ui/data-display/copyable-field.tsxRepository: tale-project/tale
Length of output: 43
🏁 Script executed:
# Check if aria-label actually overrides in practice by looking at axe rules config
cat -n services/platform/test/utils/a11y.tsRepository: tale-project/tale
Length of output: 1999
🏁 Script executed:
# Let me see what the actual accessible name would be computed as
# Check if there are any manual accessibility tests or comments about the button naming
rg -B 3 -A 3 "aria-label" services/platform/app/components/ui/data-display/copyable-field.tsxRepository: tale-project/tale
Length of output: 809
🏁 Script executed:
# Run the actual tests to see if they pass with the current implementation
cd services/platform && npm test -- copyable-field.test.tsx 2>&1 | head -100Repository: tale-project/tale
Length of output: 504
Refactor the label-button relationship to avoid losing context in the accessible name.
Currently, <Label htmlFor={valueId}> binds the label to the button, making label clicks activate the copy action. The aria-label="Copy" (or default fallback) then overrides both the visible label and the field value in the button's accessible name. Screen readers announce only "Copy" with no context about which field is being copied, and clicking the label to copy is an unexpected interaction.
Use aria-labelledby instead to compose the accessible name from the label and value, and keep the label separate. This preserves the label visually while giving the button proper context without relying on a click affordance that doesn't match user expectations.
Suggested approach
const reactId = React.useId();
+ const labelId = `${reactId}-label`;
const valueId = `${reactId}-value`;
+ const valueTextId = `${reactId}-value-text`;
const statusId = `${reactId}-status`;
- {label && <Label htmlFor={valueId}>{label}</Label>}
+ {label && <Label id={labelId}>{label}</Label>}
<button
id={valueId}
type="button"
onClick={onClick}
- aria-label={copyAriaLabel || tCommon('actions.copy')}
+ aria-labelledby={label ? `${labelId} ${valueTextId}` : valueTextId}
+ aria-label={copyAriaLabel}
aria-describedby={copied ? statusId : undefined}
>
- <span
+ <span
+ id={valueTextId}
className={cn(
'text-muted-foreground flex-1 truncate text-sm',
mono && 'font-mono',
)}🤖 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/copyable-field.tsx` around
lines 49 - 51, The Label currently uses htmlFor={valueId} and the copy Button
relies on aria-label which causes the label to bind to the button and the
accessible name to be just "Copy"; change this so the Label is not bound to the
button (remove htmlFor on Label or point it only at the actual input if present)
and set the copy Button to use aria-labelledby referencing the label and value
IDs (valueId and the label's id derived from reactId) plus statusId as needed so
the button's accessible name is composed of label + value (and status) rather
than overridden by aria-label; update references to reactId, valueId, statusId,
Label and the copy Button and remove the label-click-to-copy behavior.
| cell: ({ row }) => ( | ||
| <Text as="span" variant="muted"> | ||
| {row.original.lastIp ?? '—'} | ||
| </Text> |
There was a problem hiding this comment.
Use a translated empty-cell placeholder here.
The raw — fallback is user-visible UI, so this cell now bypasses the locale files while the rest of the table is translated. Please reuse an existing translated empty placeholder (tables.cells.empty or equivalent) for consistency.
As per coding guidelines, "No hardcoded user-facing strings. Always use the translation hook. A stray English literal in JSX is a bug."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@services/platform/app/features/settings/audit-logs/components/block-counters-table.tsx`
around lines 64 - 67, Replace the hardcoded fallback '—' in the cell renderer
with the translated empty-cell placeholder: import and use the translation hook
(e.g., useTranslation / t) in the component containing the cell renderer (the
block-counters-table component) and change the expression returning
{row.original.lastIp ?? '—'} to use t('tables.cells.empty') (or the equivalent
key) so the Text component renders the localized placeholder when lastIp is
null/undefined.
| {tSettings('organization.organizationIdHelp')} | ||
| </span> | ||
| </Stack> | ||
| <CopyableField value={organization._id} /> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Provide a context-specific copyAriaLabel for the organization ID button.
CopyableField falls back to a generic copy label. Passing a translated, specific label (e.g., “Copy organization ID”) improves screen-reader clarity when multiple copy controls are present.
As per coding guidelines: “Accessibility is Level AA, not a nice-to-have. … labelled controls … are required.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@services/platform/app/features/settings/organization/components/organization-settings.tsx`
at line 218, The CopyableField for organization._id currently uses a generic
aria label; update the JSX to pass a context-specific copyAriaLabel (e.g., "Copy
organization ID") using your app's i18n helper (for example call
t('settings.copyOrganizationId') or useTranslation() to build the string) so
screen readers can distinguish this control; modify the organization-settings
component where <CopyableField value={organization._id} /> is rendered to
include copyAriaLabel={t('...')} (and import/use the translation hook if not
already present).
| "timestamp": "Timestamp", | ||
| "user": "User", | ||
| "ipAddress": "IP Address", | ||
| "lockOut": "Lock Out", | ||
| "ipLimit": "IP Limit" |
There was a problem hiding this comment.
Use sentence case for the new block-counter headers.
"IP Address", "Lock Out", and "IP Limit" are title case, which makes the English schema drift from the repo’s translation style. Please normalize these to sentence case here so downstream locales inherit the right baseline.
As per coding guidelines, "Use sentence case in every translation."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/messages/en.json` around lines 1160 - 1164, The JSON
translation values for the keys "ipAddress", "lockOut", and "ipLimit" are in
title case; change them to sentence case to match repository translation style
by updating the values to "IP address", "Lock out", and "IP limit" respectively
(leave "timestamp" and "user" as-is), ensuring the keys "ipAddress", "lockOut",
and "ipLimit" remain unchanged so downstream locales inherit the corrected
baseline.
| }, | ||
| "logs": { | ||
| "heading": "Journaux", | ||
| "subheading": "Consultez et gérez les journaux système.", |
There was a problem hiding this comment.
Use informal French for the logs subheading
Line 1146 uses formal imperative (“Consultez”, “gérez”). Please switch to informal form.
Suggested fix
- "subheading": "Consultez et gérez les journaux système.",
+ "subheading": "Consulte et gère les journaux système.",As per coding guidelines "Use informal form across all languages — du in German, tu in French. Never Sie or vous."
📝 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.
| "subheading": "Consultez et gérez les journaux système.", | |
| "subheading": "Consulte et gère les journaux système.", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/messages/fr.json` at line 1146, Replace the formal French
imperative in the "subheading" string with the informal "tu" form; locate the
JSON key "subheading" (currently "Consultez et gérez les journaux système.") and
change its value to the informal imperative version such as "Consulte et gère
les journaux système." so it follows the project guideline to use informal
French across logs.
- CopyableField: use aria-labelledby composing label + value so screen readers announce both, instead of label-htmlFor binding that overrode the accessible name with a generic 'Copy' label. - BlockCountersTable: use translated tables.cells.empty placeholder instead of a hardcoded em dash. - Org settings: pass a context-specific copyAriaLabel for the organization ID copy button. - en.json: normalize block-counter headers to sentence case. - fr.json: switch logs subheading to informal form.
1 participant
Summary
MMM D, YYYY HH:mm.CopyableFieldas a single bordered "ID field" pill (a11y:aria-livestatus, focus ring, truncation with title); adopt it for organization ID with a helper subtitle.Buttonsize variants and letSearchInput/FilterButtonown their own height (h-9) so sizes stay consistent across the app.Test plan
/dashboard/:id/settings/logsand verify heading, category filter, and export buttons render in the headerSummary by CodeRabbit
Release Notes
New Features
Style
Tests