fix: miscellaneous UI improvements across platform#798
Conversation
…i-team assignment Redesign the document upload dialog based on new design specifications: - Replace FormDialog with base Dialog for custom layout - Add per-file upload status tracking (pending/uploading/completed/failed) - Create TeamMultiSelect component with checkbox dropdown and chip display - Show "Organization-wide" chip as default when no teams selected - Create UploadFileRow component with color-coded type badges and progress bars - Support multi-team document assignment (teamIds array) - Add success banner, progress summary, cancel/retry functionality - Auto-upload on file selection instead of manual submit - Fix layout overflow: add min-w-0 to ContentArea, overscroll-contain to DataTable - Add tests for TeamMultiSelect, UploadFileRow, and DocumentUploadDialog
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
📝 WalkthroughWalkthroughThis PR introduces a multi-file document upload system with team selection. Changes include: refactoring DocumentUploadDialog to support multi-file uploads with per-file progress tracking using a new useDocumentUpload hook, adding TeamMultiSelect and UploadFileRow components for team assignment and file status display respectively, extending the uploadFiles API to accept multiple teamIds instead of a single teamId, updating auth configuration to allow removing all teams, implementing disabled state in EntityRowActions, improving team deletion handling, and adding comprehensive test coverage for the new upload dialog and components. Additional UI refinements include spacing adjustments, icon replacements, timestamp component updates, and translation changes reflecting "upload" terminology. Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
services/platform/lib/auth-client.ts (1)
51-58:⚠️ Potential issue | 🔴 CriticalRemove
allowRemovingAllTeamsfrom client-side auth configuration.
allowRemovingAllTeamsis a server-side only option for theorganization()plugin and must not be configured in the client-sideorganizationClient()plugin. The client simply calls the normal remove-team API; the server enforces whether removing the last team is allowed. Delete lines 54 (theallowRemovingAllTeams: trueline) from this client config.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform/lib/auth-client.ts` around lines 51 - 58, The client-side organization/teams configuration in auth-client.ts incorrectly sets the server-side-only option allowRemovingAllTeams; remove the allowRemovingAllTeams: true property from the teams config object (the block passed to organizationClient()/organization() in auth-client.ts) so the client no longer sends this server-only flag — leave teams.enabled and teams.defaultTeam as-is.
🤖 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/features/documents/components/__tests__/upload-file-row.test.tsx`:
- Around line 65-83: The test is matching two "failed" occurrences (filename
suffix and error) — update the assertions to be unambiguous: keep the explicit
error assertion expect(screen.getByText('Upload failed')).toBeInTheDocument()
and replace the general expect(screen.getByText(/failed/i)) with a scoped check
that the status text appears in the file row for UploadFileRow (use the file
name from baseProps to locate the row via screen.getByText(baseProps.file.name)
and then within(...) assert the status contains "Failed"), then keep the retry
button click and onRetry expectation unchanged.
In
`@services/platform/app/features/documents/components/document-upload-dialog.tsx`:
- Around line 52-54: The component currently initializes selectedTeamIds once
using selectedTeamId and never refreshes it when the dialog is reopened; update
logic so selectedTeamIds is reset from selectedTeamId whenever the dialog
transitions open (use the open prop), e.g. add a useEffect that watches open and
selectedTeamId and calls setSelectedTeamIds(selectedTeamId ? [selectedTeamId] :
[] ) when open becomes true; apply the same pattern for the other selection
state(s) referenced around lines 88-98 to ensure defaults are resynced on
reopen.
In `@services/platform/app/features/documents/components/team-multi-select.tsx`:
- Around line 75-110: The outer trigger currently renders as a <button> that
contains chip remove <button>s (see team-multi-select component using isOpen,
setIsOpen, disabled, teams, selectedTeams, orgWideLabel and removeTeam), which
creates nested interactive controls; change the outer trigger to a non-button
container (e.g., a div with role="button" and the same
onClick/aria-expanded/aria-haspopup behavior) or render the trigger and the
chips as separate sibling elements so chip remove buttons (removeTeam) are not
children of the trigger; ensure the disabled state is enforced by preventing the
outer click (check disabled || teams.length === 0) and keep chip buttons
independently operable with their aria-labels (e.g., `Remove ${team.name}`) and
styling intact.
In `@services/platform/app/features/documents/hooks/mutations.ts`:
- Around line 146-152: The cancel/remove logic only updates React state while
uploadFiles iterates a snapshot and abortControllerRef is nulled too early,
allowing hidden files to keep uploading; fix uploadFiles by creating a local
AbortController (e.g., const batchController = new AbortController()) at the
start of the batch, assign it to abortControllerRef.current but use the local
batchController.signal for all checks and pass that signal into
uploadSingleFile, ensure uploadFiles checks batchController.signal.aborted
before starting each file and breaks if aborted, and only clear
abortControllerRef.current (set to null) after the outer loop completes; also
ensure removeTrackedFile/clearTrackedFiles still update trackedFiles state but
do not attempt to control the batch—cancellation should call
abortControllerRef.current?.abort() so the batchController stops the actual
uploads.
In `@services/platform/app/features/websites/components/website-view-dialog.tsx`:
- Around line 354-357: The ternary rendering for website.lastScannedAt can be
simplified: replace the conditional that returns either <CopyableTimestamp
date={website.lastScannedAt} preset="long" /> or
<Text>{t('viewDialog.notScannedYet')}</Text> by always rendering
CopyableTimestamp and passing the fallback string via its emptyText prop (e.g.,
emptyText={t('viewDialog.notScannedYet')}), keeping preset="long" and removing
the conditional logic around website.lastScannedAt.
In `@services/platform/app/features/websites/hooks/use-websites-table-config.tsx`:
- Around line 95-100: The cell renderer in use-websites-table-config.tsx
currently checks row.original.lastScannedAt with a truthy guard which treats 0
as missing; update the condition in the cell function that renders
CopyableTimestamp to use a nullish check (row.original.lastScannedAt != null) so
valid epoch timestamps (including 0) render correctly; modify the conditional
around CopyableTimestamp accordingly.
---
Outside diff comments:
In `@services/platform/lib/auth-client.ts`:
- Around line 51-58: The client-side organization/teams configuration in
auth-client.ts incorrectly sets the server-side-only option
allowRemovingAllTeams; remove the allowRemovingAllTeams: true property from the
teams config object (the block passed to organizationClient()/organization() in
auth-client.ts) so the client no longer sends this server-only flag — leave
teams.enabled and teams.defaultTeam as-is.
🪄 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: 4e89b83c-701a-47c3-8942-02d09bbc4798
📒 Files selected for processing (18)
services/platform/app/components/layout/content-area.tsxservices/platform/app/components/ui/data-table/data-table-empty-state.tsxservices/platform/app/components/ui/data-table/data-table.tsxservices/platform/app/components/ui/entity/entity-row-actions.tsxservices/platform/app/features/documents/components/__tests__/document-upload-dialog.test.tsxservices/platform/app/features/documents/components/__tests__/team-multi-select.test.tsxservices/platform/app/features/documents/components/__tests__/upload-file-row.test.tsxservices/platform/app/features/documents/components/document-upload-dialog.tsxservices/platform/app/features/documents/components/documents-table.tsxservices/platform/app/features/documents/components/team-multi-select.tsxservices/platform/app/features/documents/components/upload-file-row.tsxservices/platform/app/features/documents/hooks/mutations.tsservices/platform/app/features/settings/teams/components/team-row-actions.tsxservices/platform/app/features/websites/components/website-view-dialog.tsxservices/platform/app/features/websites/hooks/use-websites-table-config.tsxservices/platform/convex/auth.tsservices/platform/lib/auth-client.tsservices/platform/messages/en.json
| it('shows error state and retry button for failed files', () => { | ||
| const onRetry = vi.fn(); | ||
| render( | ||
| <UploadFileRow | ||
| {...baseProps} | ||
| status="failed" | ||
| error="Upload failed" | ||
| onRetry={onRetry} | ||
| />, | ||
| ); | ||
|
|
||
| expect(screen.getByText(/failed/i)).toBeInTheDocument(); | ||
| expect(screen.getByText('Upload failed')).toBeInTheDocument(); | ||
|
|
||
| const retryButton = screen.getByRole('button', { | ||
| name: /retry/i, | ||
| }); | ||
| fireEvent.click(retryButton); | ||
| expect(onRetry).toHaveBeenCalledOnce(); |
There was a problem hiding this comment.
Use a non-ambiguous assertion for the failed state.
Line 76 now matches both the filename suffix (— Failed) and the error message (Upload failed), which is why Test UI is failing with “Found multiple elements with the text: /failed/i”. Assert the specific error text or scope the status check to the filename row instead.
Proposed test fix
- expect(screen.getByText(/failed/i)).toBeInTheDocument();
- expect(screen.getByText('Upload failed')).toBeInTheDocument();
+ expect(screen.getByText(/test-document\.pdf/i)).toHaveTextContent(
+ /— Failed/,
+ );
+ expect(screen.getByText('Upload failed')).toBeInTheDocument();📝 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.
| it('shows error state and retry button for failed files', () => { | |
| const onRetry = vi.fn(); | |
| render( | |
| <UploadFileRow | |
| {...baseProps} | |
| status="failed" | |
| error="Upload failed" | |
| onRetry={onRetry} | |
| />, | |
| ); | |
| expect(screen.getByText(/failed/i)).toBeInTheDocument(); | |
| expect(screen.getByText('Upload failed')).toBeInTheDocument(); | |
| const retryButton = screen.getByRole('button', { | |
| name: /retry/i, | |
| }); | |
| fireEvent.click(retryButton); | |
| expect(onRetry).toHaveBeenCalledOnce(); | |
| it('shows error state and retry button for failed files', () => { | |
| const onRetry = vi.fn(); | |
| render( | |
| <UploadFileRow | |
| {...baseProps} | |
| status="failed" | |
| error="Upload failed" | |
| onRetry={onRetry} | |
| />, | |
| ); | |
| expect(screen.getByText(/test-document\.pdf/i)).toHaveTextContent( | |
| /— Failed/, | |
| ); | |
| expect(screen.getByText('Upload failed')).toBeInTheDocument(); | |
| const retryButton = screen.getByRole('button', { | |
| name: /retry/i, | |
| }); | |
| fireEvent.click(retryButton); | |
| expect(onRetry).toHaveBeenCalledOnce(); |
🧰 Tools
🪛 GitHub Check: Test UI
[failure] 76-76: app/features/documents/components/tests/upload-file-row.test.tsx > UploadFileRow > shows error state and retry button for failed files
TestingLibraryElementError: Found multiple elements with the text: /failed/i
Here are the matching elements:
Ignored nodes: comments, script, style
<span
class="min-w-0 flex-1 truncate text-xs font-medium text-red-600"
test-document.pdf
— Failed
Ignored nodes: comments, script, style
<span
class="text-[11px] leading-snug text-red-800"
Upload failed
(If this is intentional, then use the *AllBy* variant of the query (like queryAllByText, getAllByText, or findAllByText)).
Ignored nodes: comments, script, style
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@services/platform/app/features/documents/components/__tests__/upload-file-row.test.tsx`
around lines 65 - 83, The test is matching two "failed" occurrences (filename
suffix and error) — update the assertions to be unambiguous: keep the explicit
error assertion expect(screen.getByText('Upload failed')).toBeInTheDocument()
and replace the general expect(screen.getByText(/failed/i)) with a scoped check
that the status text appears in the file row for UploadFileRow (use the file
name from baseProps to locate the row via screen.getByText(baseProps.file.name)
and then within(...) assert the status contains "Failed"), then keep the retry
button click and onRetry expectation unchanged.
| const [selectedTeamIds, setSelectedTeamIds] = useState<string[]>(() => | ||
| selectedTeamId ? [selectedTeamId] : [], | ||
| ); |
There was a problem hiding this comment.
Resync the default team selection when reopening the dialog.
This state only snapshots selectedTeamId on mount and when the dialog closes itself. If the global team filter changes while the dialog stays mounted but closed, reopening it reuses the old selection and uploads to the wrong team set. Sync selectedTeamIds from selectedTeamId when open flips to true, or derive that default in an effect tied to the open transition.
Also applies to: 88-98
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@services/platform/app/features/documents/components/document-upload-dialog.tsx`
around lines 52 - 54, The component currently initializes selectedTeamIds once
using selectedTeamId and never refreshes it when the dialog is reopened; update
logic so selectedTeamIds is reset from selectedTeamId whenever the dialog
transitions open (use the open prop), e.g. add a useEffect that watches open and
selectedTeamId and calls setSelectedTeamIds(selectedTeamId ? [selectedTeamId] :
[] ) when open becomes true; apply the same pattern for the other selection
state(s) referenced around lines 88-98 to ensure defaults are resynced on
reopen.
| <button | ||
| type="button" | ||
| onClick={() => | ||
| !disabled && teams.length > 0 && setIsOpen((prev) => !prev) | ||
| } | ||
| disabled={disabled || teams.length === 0} | ||
| aria-expanded={isOpen} | ||
| aria-haspopup="listbox" | ||
| className={cn( | ||
| 'flex w-full items-center gap-1.5 rounded-lg border bg-background px-3 py-2 text-sm shadow-xs transition-colors', | ||
| isOpen | ||
| ? 'border-primary ring-2 ring-primary/20' | ||
| : 'border-border hover:border-border/80', | ||
| disabled && 'cursor-not-allowed opacity-50', | ||
| )} | ||
| > | ||
| <div className="flex flex-1 flex-wrap items-center gap-1.5"> | ||
| {selectedTeams.length === 0 ? ( | ||
| <span className="bg-muted inline-flex items-center rounded px-2 py-0.5 text-xs font-medium"> | ||
| {orgWideLabel} | ||
| </span> | ||
| ) : ( | ||
| selectedTeams.map((team) => ( | ||
| <span | ||
| key={team.id} | ||
| className="bg-muted inline-flex items-center gap-1 rounded px-2 py-0.5 text-xs font-medium" | ||
| > | ||
| {team.name} | ||
| <button | ||
| type="button" | ||
| onClick={(e) => removeTeam(team.id, e)} | ||
| className="text-muted-foreground hover:text-foreground -mr-0.5 rounded-sm" | ||
| aria-label={`Remove ${team.name}`} | ||
| > | ||
| <X className="size-3" /> | ||
| </button> |
There was a problem hiding this comment.
Don't nest chip remove buttons inside the trigger.
Line 103 renders a <button> inside the outer trigger <button>. That's invalid interactive nesting, and it makes the disabled contract unreliable because the chip controls remain separate interactive targets. Split the trigger and chip actions into separate siblings, or render the outer shell as a non-button container and disable the chip buttons explicitly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/app/features/documents/components/team-multi-select.tsx`
around lines 75 - 110, The outer trigger currently renders as a <button> that
contains chip remove <button>s (see team-multi-select component using isOpen,
setIsOpen, disabled, teams, selectedTeams, orgWideLabel and removeTeam), which
creates nested interactive controls; change the outer trigger to a non-button
container (e.g., a div with role="button" and the same
onClick/aria-expanded/aria-haspopup behavior) or render the trigger and the
chips as separate sibling elements so chip remove buttons (removeTeam) are not
children of the trigger; ensure the disabled state is enforced by preventing the
outer click (check disabled || teams.length === 0) and keep chip buttons
independently operable with their aria-labels (e.g., `Remove ${team.name}`) and
styling intact.
| const removeTrackedFile = useCallback((fileId: string) => { | ||
| setTrackedFiles((prev) => prev.filter((f) => f.id !== fileId)); | ||
| }, []); | ||
|
|
||
| const totalBytes = files.reduce((sum, f) => sum + f.size, 0); | ||
| perFileLoadedRef.current = new Array(files.length).fill(0); | ||
| setUploadProgress({ | ||
| completedFiles: 0, | ||
| totalFiles: files.length, | ||
| bytesLoaded: 0, | ||
| bytesTotal: totalBytes, | ||
| }); | ||
| const clearTrackedFiles = useCallback(() => { | ||
| setTrackedFiles([]); | ||
| }, []); |
There was a problem hiding this comment.
Cancel/removal only hides queued files; the batch can keep running.
uploadFiles iterates the newTracked snapshot, while removeTrackedFile/clearTrackedFiles only mutate React state. On top of that, Line 399 clears abortControllerRef.current immediately, so the next if (abortControllerRef.current?.signal.aborted) check sees null and proceeds with the remaining files. After a user clicks Cancel—or removes pending rows—the hidden files can still upload in the background. Keep a stable controller local to the batch, pass that signal through uploadSingleFile, and only clear the ref once the outer loop has exited.
Also applies to: 298-320, 396-401
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/app/features/documents/hooks/mutations.ts` around lines 146
- 152, The cancel/remove logic only updates React state while uploadFiles
iterates a snapshot and abortControllerRef is nulled too early, allowing hidden
files to keep uploading; fix uploadFiles by creating a local AbortController
(e.g., const batchController = new AbortController()) at the start of the batch,
assign it to abortControllerRef.current but use the local batchController.signal
for all checks and pass that signal into uploadSingleFile, ensure uploadFiles
checks batchController.signal.aborted before starting each file and breaks if
aborted, and only clear abortControllerRef.current (set to null) after the outer
loop completes; also ensure removeTrackedFile/clearTrackedFiles still update
trackedFiles state but do not attempt to control the batch—cancellation should
call abortControllerRef.current?.abort() so the batchController stops the actual
uploads.
| const contentHash = await calculateFileHash(file); | ||
|
|
||
| // Step 2: Get upload URL from Convex | ||
| const uploadUrl = await generateUploadUrl({}); | ||
|
|
||
| // Step 3: Upload file with progress tracking | ||
| const { storageId } = await uploadWithProgress( | ||
| uploadUrl, | ||
| file, | ||
| resolvedType, | ||
| abortControllerRef.current?.signal, | ||
| (loaded) => { | ||
| perFileLoadedRef.current[fileIndex] = loaded; | ||
| const bytesLoaded = perFileLoadedRef.current.reduce( | ||
| (sum, v) => sum + v, | ||
| 0, | ||
| ); | ||
| setUploadProgress((prev) => | ||
| prev ? { ...prev, bytesLoaded } : prev, | ||
| ); | ||
| (loaded, total) => { | ||
| updateFileStatus(fileId, { | ||
| bytesLoaded: loaded, | ||
| bytesTotal: total, | ||
| }); | ||
| }, | ||
| ); | ||
|
|
||
| // Step 4: Create document record in database | ||
| const result = await createDocumentFromUpload({ | ||
| organizationId: options.organizationId, | ||
| fileId: toId<'_storage'>(storageId), | ||
| fileName: file.name, | ||
| contentType: resolvedType, | ||
| contentHash, | ||
| metadata: { | ||
| size: file.size, | ||
| sourceProvider: 'upload', | ||
| sourceMode: 'manual', | ||
| }, | ||
| teamId: uploadOptions?.teamId, | ||
| folderId: uploadOptions?.folderId | ||
| ? toId<'folders'>(uploadOptions.folderId) | ||
| : undefined, | ||
| fileSize: file.size, | ||
| }); | ||
| // Create document records — one per team, or one org-wide | ||
| const teamIds = uploadOptions?.teamIds; | ||
| if (teamIds && teamIds.length > 0) { | ||
| for (const teamId of teamIds) { | ||
| await createDocumentFromUpload({ | ||
| organizationId: options.organizationId, | ||
| fileId: toId<'_storage'>(storageId), | ||
| fileName: file.name, | ||
| contentType: resolvedType, | ||
| contentHash, | ||
| metadata: { | ||
| size: file.size, | ||
| sourceProvider: 'upload', | ||
| sourceMode: 'manual', | ||
| }, | ||
| teamId, | ||
| folderId: uploadOptions?.folderId | ||
| ? toId<'folders'>(uploadOptions.folderId) | ||
| : undefined, | ||
| fileSize: file.size, | ||
| }); | ||
| } | ||
| } else { | ||
| await createDocumentFromUpload({ | ||
| organizationId: options.organizationId, | ||
| fileId: toId<'_storage'>(storageId), | ||
| fileName: file.name, | ||
| contentType: resolvedType, | ||
| contentHash, | ||
| metadata: { | ||
| size: file.size, | ||
| sourceProvider: 'upload', | ||
| sourceMode: 'manual', | ||
| }, | ||
| teamId: undefined, | ||
| folderId: uploadOptions?.folderId | ||
| ? toId<'folders'>(uploadOptions.folderId) | ||
| : undefined, | ||
| fileSize: file.size, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Multi-team retries are not idempotent.
After the blob is stored, Lines 185-204 fan out createDocumentFromUpload one team at a time from the client. If one team creation succeeds and a later one fails, the file is marked failed; retrying then uploads a second blob and replays all team creates. That can duplicate documents for teams that already succeeded and orphan the first storage object. The team fan-out needs to happen behind a single server-side mutation, or the mutation needs an idempotency key covering the uploaded content and target team/folder.
| value: website.lastScannedAt ? ( | ||
| <CopyableTimestamp date={website.lastScannedAt} preset="long" /> | ||
| ) : ( | ||
| <Text>{t('viewDialog.notScannedYet')}</Text> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Optional: simplify lastScannedAt branch with emptyText.
You can remove the conditional and keep one rendering path by delegating fallback text to CopyableTimestamp.
♻️ Proposed refactor
- value: website.lastScannedAt ? (
- <CopyableTimestamp date={website.lastScannedAt} preset="long" />
- ) : (
- <Text>{t('viewDialog.notScannedYet')}</Text>
- ),
+ value: (
+ <CopyableTimestamp
+ date={website.lastScannedAt ?? null}
+ preset="long"
+ emptyText={t('viewDialog.notScannedYet')}
+ />
+ ),📝 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.
| value: website.lastScannedAt ? ( | |
| <CopyableTimestamp date={website.lastScannedAt} preset="long" /> | |
| ) : ( | |
| <Text>{t('viewDialog.notScannedYet')}</Text> | |
| value: ( | |
| <CopyableTimestamp | |
| date={website.lastScannedAt ?? null} | |
| preset="long" | |
| emptyText={t('viewDialog.notScannedYet')} | |
| /> | |
| ), |
🤖 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 354 - 357, The ternary rendering for website.lastScannedAt can be
simplified: replace the conditional that returns either <CopyableTimestamp
date={website.lastScannedAt} preset="long" /> or
<Text>{t('viewDialog.notScannedYet')}</Text> by always rendering
CopyableTimestamp and passing the fallback string via its emptyText prop (e.g.,
emptyText={t('viewDialog.notScannedYet')}), keeping preset="long" and removing
the conditional logic around website.lastScannedAt.
| cell: ({ row }) => | ||
| row.original.lastScannedAt ? ( | ||
| <TableDateCell | ||
| date={row.original.lastScannedAt} | ||
| preset="short" | ||
| <CopyableTimestamp | ||
| date={row.original.lastScannedAt} | ||
| preset="long" | ||
| alignRight |
There was a problem hiding this comment.
Use a nullish check for lastScannedAt instead of a truthy check.
A truthy guard will treat 0 as missing and show the loading spinner. Use != null so valid epoch timestamps still render.
Suggested fix
- cell: ({ row }) =>
- row.original.lastScannedAt ? (
+ cell: ({ row }) =>
+ row.original.lastScannedAt != null ? (
<CopyableTimestamp
date={row.original.lastScannedAt}
preset="long"
alignRight
/>
) : (📝 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.
| cell: ({ row }) => | |
| row.original.lastScannedAt ? ( | |
| <TableDateCell | |
| date={row.original.lastScannedAt} | |
| preset="short" | |
| <CopyableTimestamp | |
| date={row.original.lastScannedAt} | |
| preset="long" | |
| alignRight | |
| cell: ({ row }) => | |
| row.original.lastScannedAt != null ? ( | |
| <CopyableTimestamp | |
| date={row.original.lastScannedAt} | |
| preset="long" | |
| alignRight | |
| /> | |
| ) : ( |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/app/features/websites/hooks/use-websites-table-config.tsx`
around lines 95 - 100, The cell renderer in use-websites-table-config.tsx
currently checks row.original.lastScannedAt with a truthy guard which treats 0
as missing; update the condition in the cell function that renders
CopyableTimestamp to use a nullish check (row.original.lastScannedAt != null) so
valid epoch timestamps (including 0) render correctly; modify the conditional
around CopyableTimestamp accordingly.
Israeltheminer
force-pushed
the
fix/misc-ui-improvements
branch
from
3c8da24 to
436bbcd
Compare
- Reduce empty state icon size from size-10 to size-6 and add gap-2 - Add disabled prop to EntityRowActions to prevent interaction after deletion - Disable team row actions after team deletion to prevent stale operations - Change documents empty state icon from ClipboardList to FileText - Fix website view dialog: add chunk content scroll, grid layout for pages - Use CopyableTimestamp for website last scanned date - Allow removing all teams in auth config (allowRemovingAllTeams)
Israeltheminer
force-pushed
the
fix/misc-ui-improvements
branch
from
436bbcd to
775a2c9
Compare
1 participant
Summary
disabledprop toEntityRowActionsto prevent interaction after deletionClipboardListtoFileTextTableDateCellwithCopyableTimestampfor website last scanned dateallowRemovingAllTeamsin auth config for org-wide document supportTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Improvements