Knowledge base: Creation and deletion UI #28
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a client-side Knowledge Base UI, multiple Next.js API proxy routes for collections and jobs that forward Changes
Sequence DiagramsequenceDiagram
participant Client as Client/Browser
participant UI as KnowledgeBase UI
participant APIRoute as Next.js API Route
participant Backend as Backend Service
participant Poller as Polling Component
Client->>UI: User creates collection (form, X-API-KEY from localStorage)
UI->>APIRoute: POST /api/collections (X-API-KEY, body)
APIRoute->>Backend: POST /api/v1/collections (forward key & body)
Backend-->>APIRoute: 200 { job_id }
APIRoute-->>UI: 200 { job_id } (optimistic entry created)
loop polling until job complete
Poller->>APIRoute: GET /api/collections/jobs/{job_id} (X-API-KEY)
APIRoute->>Backend: GET /api/v1/collections/jobs/{job_id}
Backend-->>APIRoute: job status
APIRoute-->>Poller: job status
Poller->>UI: update collection status / reconcile optimistic entry
end
Client->>UI: View collection details
UI->>APIRoute: GET /api/collections/{collection_id} (X-API-KEY)
APIRoute->>Backend: GET /api/v1/collections/{collection_id}?include_docs=true
Backend-->>APIRoute: collection + docs
APIRoute-->>UI: collection payload
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Suggested reviewers
Poem
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@app/components/utils.ts`:
- Around line 10-21: The formatDate function currently assumes new
Date(dateString) is valid; update formatDate to guard against invalid dates by
checking Number.isNaN(date.getTime()) (or isNaN(date.getTime())) right after
creating const date = new Date(dateString) and before applying the IST offset
and formatting; if the check fails return 'N/A' (or the current fallback) so you
never format an Invalid Date and produce NaN values. Ensure you keep the
existing IST conversion and formatting logic in formatDate unchanged except for
this validity guard.
In `@app/knowledge-base/page.tsx`:
- Around line 170-188: The fetchCollections function assumes the API returns an
object with result.data, so when /api/collections returns a raw array the UI
shows zero items; change the parsing to accept both shapes by treating the
response as const collections = result.data || result || [] before
enriching—then continue to call enrichCollectionWithCache on that array and pass
the enrichedCollections to setCollections.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@app/knowledge-base/page.tsx`:
- Around line 598-609: The onChange handler for the batch size input can set
batchSize to NaN because parseInt(e.target.value) may return NaN; update the
handler used with setBatchSize so you first coerce/validate the parsed value
(e.g., const n = parseInt(e.target.value); const valid = Number.isNaN(n) ? 1 :
n) and then call setBatchSize(Math.max(1, valid)) — modify the inline onChange
arrow function that currently uses setBatchSize(Math.max(1,
parseInt(e.target.value) || 1)) to ensure NaN is handled explicitly before
Math.max.
- Around line 270-275: In handleConfirmCreate add an explicit null guard for
apiKey before using apiKey.key: check if apiKey is null (e.g., if (!apiKey) {
handle the error/return early }) and only call fetch('/api/collections', ...)
when apiKey is non-null; update any UI state (close modal or set an error) in
the early-return branch so TypeScript no longer complains about accessing
apiKey.key and future regressions are prevented.
🧹 Nitpick comments (4)
app/knowledge-base/page.tsx (4)
9-17: DuplicateDocumentinterface definition.This interface is identical to the one in
app/document/page.tsx. Consider importing it from there or extracting to a shared types file to avoid drift and reduce maintenance overhead.-export interface Document { - id: string; - fname: string; - object_store_url: string; - signed_url?: string; - file_size?: number; - inserted_at?: string; - updated_at?: string; -} +import { Document } from '@/app/document/page';
79-141: Sequential API calls inenrichCollectionWithCachemay cause slow loading.This function is called via
Promise.allon line 185-186, but internally it performs sequential API calls within loops (lines 99, 120). For users with many collections, this creates a waterfall of network requests during enrichment.Consider batching job status requests or fetching all job statuses in a single call if the backend supports it, then matching locally.
367-372: Missing dependencies inuseEffectmay cause stale closures.The effect depends on
apiKeybut callsfetchCollectionsandfetchDocumentswhich are recreated on each render. Either add them to the dependency array withuseCallback, or inline the fetch logic.+ const fetchCollections = useCallback(async () => { + // ... existing implementation + }, [apiKey]); + + const fetchDocuments = useCallback(async () => { + // ... existing implementation + }, [apiKey]); + useEffect(() => { if (apiKey) { fetchCollections(); fetchDocuments(); } - }, [apiKey]); + }, [apiKey, fetchCollections, fetchDocuments]);Alternatively, suppress the lint warning if the current behavior is intentional, but document why.
707-711: Use theme color instead of hardcoded hex value.The delete button uses hardcoded
#ef4444while the project hascolors.status.error(#dc2626) defined in the theme. Consider using the theme color for consistency.style={{ backgroundColor: 'transparent', - color: '#ef4444', - border: '1px solid `#ef4444`', + color: colors.status.error, + border: `1px solid ${colors.status.error}`, }}
nishika26
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@app/knowledge-base/page.tsx`:
- Around line 631-635: The click handler currently always calls
fetchCollectionDetails(collection.id) which triggers failing API calls for
optimistic collections whose ids start with "optimistic-"; update the onClick
logic in the component using setShowCreateForm and setShowDocumentPicker so it
only calls fetchCollectionDetails(collection.id) when collection.id does NOT
startWith "optimistic-"; leave the state updates (setShowCreateForm(false),
setShowDocumentPicker(false)) unchanged and avoid making the network call for
optimistic entries to prevent errors and flicker.
- Around line 667-690: The delete button currently appears for optimistic
(pending) collections and triggers handleDeleteCollection(collection.id) which
calls the DELETE API with an optimistic-* id and fails; update the render logic
to suppress the button for optimistic entries (e.g., when collection.id
startsWith("optimistic-") or collection.pending is true) or change
handleDeleteCollection to detect optimistic ids and remove the collection from
local state without calling the API; locate the button block that references
handleDeleteCollection and collection.id and add the conditional to hide the
button (or route to a local-delete branch) so optimistic collections are not
sent to the DELETE endpoint.
🧹 Nitpick comments (5)
app/knowledge-base/page.tsx (5)
9-17: Consider consolidating the duplicateDocumentinterface.This
Documentinterface is identical to the one exported fromapp/document/page.tsx. Duplicating type definitions can lead to drift over time. Consider importing from a shared types file or directly from the document page.-export interface Document { - id: string; - fname: string; - object_store_url: string; - signed_url?: string; - file_size?: number; - inserted_at?: string; - updated_at?: string; -} +import { Document } from '../document/page';Alternatively, create a shared types module (e.g.,
app/types/index.ts) to house common interfaces.
94-119: Sequential API calls in loop may degrade performance.When the cache has many entries without
collection_id, this loop makes sequentialfetchJobStatuscalls for each one. With a large cache, this can significantly slow down the initial load.Consider batching these lookups or limiting the number of iterations to avoid excessive API calls during enrichment.
275-333: Polling iterations may overlap if API calls take longer than 3 seconds.The
setIntervalcallback is async, butsetIntervaldoesn't wait for the previous iteration to complete. If network requests are slow, multiple polling cycles could run concurrently, causing redundant API calls and potential race conditions when updating state.Consider adding a guard to skip the iteration if the previous one is still in progress:
Proposed fix
+ const isPollingRef = useRef(false); + pollingRef.current = setInterval(async () => { + if (isPollingRef.current) return; + isPollingRef.current = true; + try { const currentApiKey = apiKeyRef.current; // ... existing logic ... + } finally { + isPollingRef.current = false; + } }, 3000);
496-501: useEffect has missing dependencies that may cause linter warnings.
fetchCollectionsandfetchDocumentsare called inside this effect but are not listed as dependencies. Since these functions are recreated on each render, adding them would cause infinite loops. Consider wrapping them withuseCallbackto stabilize their references, or add an ESLint disable comment if intentional.+ // eslint-disable-next-line react-hooks/exhaustive-deps useEffect(() => { if (apiKey) { fetchCollections(); fetchDocuments(); } }, [apiKey]);
1135-1141: Consider addingsandboxattribute to the iframe for defense-in-depth.The iframe loads document content from signed URLs. Adding a
sandboxattribute restricts the embedded content's capabilities, providing an additional security layer.<iframe key={previewDoc.id} src={previewDoc.signed_url} title={previewDoc.fname} className="w-full h-full" - style={{ border: 'none' }} + style={{ border: 'none' }} + sandbox="allow-same-origin allow-scripts" />Note: Adjust sandbox permissions based on what the previewed documents require (e.g.,
allow-scriptsonly if needed for PDF viewers).
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@app/knowledge-base/page.tsx`:
- Around line 286-362: startPolling's interval captures a stale fetchCollections
function; replace the direct call to fetchCollections() with a ref-backed
invocation (e.g., create fetchCollectionsRef.current and assign fetchCollections
to it wherever it's defined) or inline the refresh logic using apiKeyRef so the
interval always uses the latest values. Update references: in startPolling (and
where fetchCollections is defined/assigned) ensure you expose fetchCollections
via fetchCollectionsRef (or move its body into the interval using apiKeyRef) and
call fetchCollectionsRef.current() instead of fetchCollections() so the interval
no longer closes over an outdated function.
- Around line 457-459: The error path in the else branch of the
create-knowledge-base flow calls response.json() unguarded (in
app/knowledge-base/page.tsx) which can throw on non-JSON responses; update that
code where response.json() is called so it safely parses with a fallback (e.g.,
await response.json().catch(() => ({})) or wrap in try/catch) and include the
HTTP status in the alert/log (use response.status and response.statusText) when
constructing the message so you retain HTTP context; look for the exact
unguarded call to response.json() in the create knowledge base handler to apply
this change.
- Line 334: The isComplete calculation is using mismatched case causing every
job to be treated as complete; update the isComplete expression (the line that
defines isComplete using status.toLowerCase()) to check against a lowercase
array of terminal/non-waiting states (e.g.
['pending','processing','in_progress'] or the same set used at the other
location) so that status.toLowerCase() can match correctly and prevent premature
removal from the polling map (refer to the isComplete variable and the status
variable in this file).
🧹 Nitpick comments (3)
app/knowledge-base/page.tsx (3)
9-17:Documentinterface is duplicated fromapp/document/page.tsx.Both files export an identical
Documentinterface. Consider extracting it into a shared types file (e.g.,app/types.ts) to avoid drift.
86-148:enrichCollectionWithCachefires N×M network requests on every fetch.Each collection triggers up to one
fetchJobStatuscall per uncached job entry in localStorage (lines 100-121), plus another ifjobIdis found but status is missing (lines 125-139). With many collections and cache entries, this fans out into a large number of sequential-per-collection, parallel-across-collections API calls on everyfetchCollectionsinvocation. Consider short-circuiting enrichment for collections that already have a terminal status cached, or batch-fetching job statuses.
984-1022: Modals lack Escape-key handling and focus trapping.The four modals (confirm create, confirm delete, document picker, document preview) don't listen for the Escape key or trap focus. The
UploadDocumentModalinapp/document/page.tsxalready implements Escape handling — consider applying the same pattern here for consistency and accessibility.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@app/knowledge-base/page.tsx`:
- Around line 86-148: enrichCollectionWithCache currently loops all cached jobs
and calls fetchJobStatus per collection, causing an N×M request burst; instead,
before calling Promise.all over collections, collect the set of uncached job IDs
from the local CACHE_KEY, call fetchJobStatus once per job to build a
jobStatusMap, and then change enrichCollectionWithCache (or its callers) to use
that jobStatusMap for lookups rather than iterating and fetching inside the
function; keep saveCollectionData and fetchJobStatus usages but replace
per-collection cache-iteration with a simple map lookup (or add a new parameter
jobStatusMap to enrichCollectionWithCache) so each job is fetched at most once
per cycle.
- Around line 205-215: The updater passed to setSelectedCollection contains a
side effect (calling fetchCollectionDetails) which must be removed; instead
detect and perform the optimistic->real swap and the fetch outside the state
updater (for example, move the logic into a useEffect that watches
enrichedCollections and selectedCollection), where you find the replacement via
enrichedCollections.find(c => c.job_id === selectedCollection?.job_id), call
setSelectedCollection(replacement) (purely) and then call
fetchCollectionDetails(replacement.id) if a replacement was found; ensure
setSelectedCollection's functional updater no longer calls
fetchCollectionDetails and only returns the new state.
- Around line 986-1027: The delete-confirmation modal rendered when
showConfirmDelete is true needs ARIA roles and keyboard handling: add
role="dialog" and aria-modal="true" to the modal container, set an initial
focusable element (e.g., the Cancel or Delete button) via a ref, implement an
Escape key handler that calls setShowConfirmDelete(false) and clears
setCollectionToDelete(null), and implement a focus-trap that captures
Tab/Shift+Tab to cycle focus among modal focusable elements (use a useEffect in
the component that adds/removes a keydown listener which prevents focus leaving
the modal and restores focus on unmount). Update handlers like
handleConfirmDelete to still close the modal after action; apply the same
pattern later to the document picker and preview modals.
🧹 Nitpick comments (5)
app/knowledge-base/page.tsx (5)
7-7: Importing types from another page component creates tight coupling.
APIKeyandSTORAGE_KEYare implementation details of the keystore page. If that page is refactored, this import breaks. Consider extracting shared types/constants into a dedicated module (e.g.,app/lib/keystore.ts).
541-546: Missing dependencies in effect —fetchCollectionsandfetchDocumentsare not stable.
fetchCollectionsandfetchDocumentsare recreated every render (not wrapped inuseCallback), so omitting them from the dependency array silences the exhaustive-deps lint rule but means the effect captures a stale closure on the initial render. This works today only because both functions readapiKeyfrom component scope and the effect re-runs whenapiKeychanges.Consider wrapping both in
useCallback(with[apiKey]deps) or using refs consistently as you've done forfetchCollectionsRef.
421-421: Hardcodedprovider: 'openai'.This value is not configurable by the user. If additional providers are supported in the future, this will need to change. Consider making it a form field or a configuration constant.
30-1211: Consider splitting this ~1200-line component into smaller pieces.This single component handles data fetching, polling, caching, optimistic updates, and renders multiple modals + list/detail views. Extracting logical units (e.g.,
CollectionList,CreateCollectionForm,DocumentPickerModal, custom hooks for polling and collection CRUD) would improve readability and testability.
56-67: localStorage cache is never pruned.Entries in
collection_job_cacheare added on create but never removed on delete or after jobs complete. Over time this grows unbounded. Consider cleaning up entries when a collection is deleted or when the job reaches a terminal state.
| const enrichCollectionWithCache = async (collection: Collection): Promise<Collection> => { | ||
| // First try to look up cached data by collection_id | ||
| let cached = getCollectionDataByCollectionId(collection.id); | ||
|
|
||
| let jobId = cached.job_id; | ||
| let collectionJobStatus = null; | ||
| let name = cached.name; | ||
| let description = cached.description; | ||
|
|
||
| // If we don't have cached data by collection_id, we need to find it by checking all jobs | ||
| if (!jobId && apiKey) { | ||
| const cache = JSON.parse(localStorage.getItem(CACHE_KEY) || '{}'); | ||
|
|
||
| // Try each job_id in the cache to find which one matches this collection | ||
| for (const [cachedJobId, data] of Object.entries(cache)) { | ||
| const cacheData = data as { name: string; description: string; collection_id?: string }; | ||
|
|
||
| // If collection_id is not set yet, fetch job info to check | ||
| if (!cacheData.collection_id) { | ||
| try { | ||
| const jobInfo = await fetchJobStatus(cachedJobId); | ||
| if (jobInfo?.collectionId === collection.id) { | ||
| jobId = cachedJobId; | ||
| name = cacheData.name; | ||
| description = cacheData.description; | ||
| collectionJobStatus = jobInfo.status; | ||
|
|
||
| // Update cache with collection_id for faster lookup next time | ||
| saveCollectionData(cachedJobId, cacheData.name, cacheData.description, collection.id); | ||
| break; | ||
| } | ||
| } catch (error) { | ||
| console.error('Error checking job:', cachedJobId, error); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // If we have job_id but no status yet, fetch it | ||
| if (jobId && !collectionJobStatus && apiKey) { | ||
| try { | ||
| const jobInfo = await fetchJobStatus(jobId); | ||
| if (jobInfo?.status) { | ||
| collectionJobStatus = jobInfo.status; | ||
|
|
||
| // Update cache with collection_id if not already set | ||
| if (jobInfo.collectionId && !cached.job_id) { | ||
| saveCollectionData(jobId, name || '', description || '', jobInfo.collectionId); | ||
| } | ||
| } | ||
| } catch (error) { | ||
| console.error('Failed to fetch live status:', error); | ||
| } | ||
| } | ||
|
|
||
| return { | ||
| ...collection, | ||
| name: name || 'Untitled Collection', | ||
| description: description || '', | ||
| status: collectionJobStatus || undefined, | ||
| job_id: jobId, | ||
| }; | ||
| }; |
There was a problem hiding this comment.
Potential N×M network request burst during enrichment.
enrichCollectionWithCache is called via Promise.all for every collection (line 192-194). For each collection without a cached collection_id, it iterates all uncached job entries and calls fetchJobStatus for each (lines 100-120). With N collections and M uncached jobs, this fans out to up to N×M concurrent requests on every fetch cycle.
Consider pre-fetching all job statuses once into a map before enriching, so each job is fetched at most once per cycle.
🤖 Prompt for AI Agents
In `@app/knowledge-base/page.tsx` around lines 86 - 148, enrichCollectionWithCache
currently loops all cached jobs and calls fetchJobStatus per collection, causing
an N×M request burst; instead, before calling Promise.all over collections,
collect the set of uncached job IDs from the local CACHE_KEY, call
fetchJobStatus once per job to build a jobStatusMap, and then change
enrichCollectionWithCache (or its callers) to use that jobStatusMap for lookups
rather than iterating and fetching inside the function; keep saveCollectionData
and fetchJobStatus usages but replace per-collection cache-iteration with a
simple map lookup (or add a new parameter jobStatusMap to
enrichCollectionWithCache) so each job is fetched at most once per cycle.
| setSelectedCollection((prev) => { | ||
| if (prev?.id.startsWith('optimistic-') && prev.job_id) { | ||
| const replacement = enrichedCollections.find((c: Collection) => c.job_id === prev.job_id); | ||
| if (replacement) { | ||
| // Fetch full details including documents for the real collection | ||
| fetchCollectionDetails(replacement.id); | ||
| return replacement; | ||
| } | ||
| } | ||
| return prev; | ||
| }); |
There was a problem hiding this comment.
Side effect (fetchCollectionDetails) inside a state updater function.
setSelectedCollection updater (line 205) calls fetchCollectionDetails (line 210), which triggers a fetch and further setState calls. State updaters should be pure functions. This can cause unexpected re-render cascading and is hard to reason about.
Move the swap-and-fetch logic outside the updater.
Proposed fix sketch
- setSelectedCollection((prev) => {
- if (prev?.id.startsWith('optimistic-') && prev.job_id) {
- const replacement = enrichedCollections.find((c: Collection) => c.job_id === prev.job_id);
- if (replacement) {
- fetchCollectionDetails(replacement.id);
- return replacement;
- }
- }
- return prev;
- });
+ // Swap optimistic → real selection outside the updater
+ const currentSelected = selectedCollection;
+ if (currentSelected?.id.startsWith('optimistic-') && currentSelected.job_id) {
+ const replacement = enrichedCollections.find((c: Collection) => c.job_id === currentSelected.job_id);
+ if (replacement) {
+ setSelectedCollection(replacement);
+ fetchCollectionDetails(replacement.id);
+ }
+ }📝 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.
| setSelectedCollection((prev) => { | |
| if (prev?.id.startsWith('optimistic-') && prev.job_id) { | |
| const replacement = enrichedCollections.find((c: Collection) => c.job_id === prev.job_id); | |
| if (replacement) { | |
| // Fetch full details including documents for the real collection | |
| fetchCollectionDetails(replacement.id); | |
| return replacement; | |
| } | |
| } | |
| return prev; | |
| }); | |
| // Swap optimistic → real selection outside the updater | |
| const currentSelected = selectedCollection; | |
| if (currentSelected?.id.startsWith('optimistic-') && currentSelected.job_id) { | |
| const replacement = enrichedCollections.find((c: Collection) => c.job_id === currentSelected.job_id); | |
| if (replacement) { | |
| setSelectedCollection(replacement); | |
| fetchCollectionDetails(replacement.id); | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@app/knowledge-base/page.tsx` around lines 205 - 215, The updater passed to
setSelectedCollection contains a side effect (calling fetchCollectionDetails)
which must be removed; instead detect and perform the optimistic->real swap and
the fetch outside the state updater (for example, move the logic into a
useEffect that watches enrichedCollections and selectedCollection), where you
find the replacement via enrichedCollections.find(c => c.job_id ===
selectedCollection?.job_id), call setSelectedCollection(replacement) (purely)
and then call fetchCollectionDetails(replacement.id) if a replacement was found;
ensure setSelectedCollection's functional updater no longer calls
fetchCollectionDetails and only returns the new state.
| {showConfirmDelete && ( | ||
| <div className="fixed inset-0 bg-black bg-opacity-50 flex items-center justify-center z-50"> | ||
| <div | ||
| className="rounded-lg p-6 w-full max-w-md" | ||
| style={{ backgroundColor: colors.bg.primary }} | ||
| > | ||
| <h2 className="text-xl font-semibold mb-4" style={{ color: colors.text.primary }}> | ||
| Delete Collection | ||
| </h2> | ||
| <p className="text-sm mb-6" style={{ color: colors.text.secondary }}> | ||
| Are you sure you want to delete this collection? This action cannot be undone. | ||
| </p> | ||
| <div className="flex justify-end gap-3"> | ||
| <button | ||
| onClick={() => { | ||
| setShowConfirmDelete(false); | ||
| setCollectionToDelete(null); | ||
| }} | ||
| className="px-4 py-2 rounded-md text-sm font-medium" | ||
| style={{ | ||
| backgroundColor: 'transparent', | ||
| color: colors.text.secondary, | ||
| border: `1px solid ${colors.border}`, | ||
| }} | ||
| > | ||
| Cancel | ||
| </button> | ||
| <button | ||
| onClick={handleConfirmDelete} | ||
| className="px-4 py-2 rounded-md text-sm font-medium" | ||
| style={{ | ||
| backgroundColor: '#ef4444', | ||
| color: '#ffffff', | ||
| border: '1px solid #ef4444', | ||
| }} | ||
| > | ||
| Delete | ||
| </button> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
Modals lack keyboard accessibility and ARIA roles.
The confirmation modal (and document picker / preview modals) is missing:
role="dialog"andaria-modal="true"attributes- Focus trapping (Tab should cycle within the modal)
- Escape key handler to dismiss
This impacts keyboard-only and screen-reader users.
Minimal improvement for the delete confirmation modal
- <div className="fixed inset-0 bg-black bg-opacity-50 flex items-center justify-center z-50">
+ <div
+ className="fixed inset-0 bg-black bg-opacity-50 flex items-center justify-center z-50"
+ role="dialog"
+ aria-modal="true"
+ onKeyDown={(e) => {
+ if (e.key === 'Escape') {
+ setShowConfirmDelete(false);
+ setCollectionToDelete(null);
+ }
+ }}
+ >Apply similar changes to the document picker and preview modals.
📝 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.
| {showConfirmDelete && ( | |
| <div className="fixed inset-0 bg-black bg-opacity-50 flex items-center justify-center z-50"> | |
| <div | |
| className="rounded-lg p-6 w-full max-w-md" | |
| style={{ backgroundColor: colors.bg.primary }} | |
| > | |
| <h2 className="text-xl font-semibold mb-4" style={{ color: colors.text.primary }}> | |
| Delete Collection | |
| </h2> | |
| <p className="text-sm mb-6" style={{ color: colors.text.secondary }}> | |
| Are you sure you want to delete this collection? This action cannot be undone. | |
| </p> | |
| <div className="flex justify-end gap-3"> | |
| <button | |
| onClick={() => { | |
| setShowConfirmDelete(false); | |
| setCollectionToDelete(null); | |
| }} | |
| className="px-4 py-2 rounded-md text-sm font-medium" | |
| style={{ | |
| backgroundColor: 'transparent', | |
| color: colors.text.secondary, | |
| border: `1px solid ${colors.border}`, | |
| }} | |
| > | |
| Cancel | |
| </button> | |
| <button | |
| onClick={handleConfirmDelete} | |
| className="px-4 py-2 rounded-md text-sm font-medium" | |
| style={{ | |
| backgroundColor: '#ef4444', | |
| color: '#ffffff', | |
| border: '1px solid #ef4444', | |
| }} | |
| > | |
| Delete | |
| </button> | |
| </div> | |
| </div> | |
| </div> | |
| )} | |
| {showConfirmDelete && ( | |
| <div | |
| className="fixed inset-0 bg-black bg-opacity-50 flex items-center justify-center z-50" | |
| role="dialog" | |
| aria-modal="true" | |
| onKeyDown={(e) => { | |
| if (e.key === 'Escape') { | |
| setShowConfirmDelete(false); | |
| setCollectionToDelete(null); | |
| } | |
| }} | |
| > | |
| <div | |
| className="rounded-lg p-6 w-full max-w-md" | |
| style={{ backgroundColor: colors.bg.primary }} | |
| > | |
| <h2 className="text-xl font-semibold mb-4" style={{ color: colors.text.primary }}> | |
| Delete Collection | |
| </h2> | |
| <p className="text-sm mb-6" style={{ color: colors.text.secondary }}> | |
| Are you sure you want to delete this collection? This action cannot be undone. | |
| </p> | |
| <div className="flex justify-end gap-3"> | |
| <button | |
| onClick={() => { | |
| setShowConfirmDelete(false); | |
| setCollectionToDelete(null); | |
| }} | |
| className="px-4 py-2 rounded-md text-sm font-medium" | |
| style={{ | |
| backgroundColor: 'transparent', | |
| color: colors.text.secondary, | |
| border: `1px solid ${colors.border}`, | |
| }} | |
| > | |
| Cancel | |
| </button> | |
| <button | |
| onClick={handleConfirmDelete} | |
| className="px-4 py-2 rounded-md text-sm font-medium" | |
| style={{ | |
| backgroundColor: '#ef4444', | |
| color: '#ffffff', | |
| border: '1px solid `#ef4444`', | |
| }} | |
| > | |
| Delete | |
| </button> | |
| </div> | |
| </div> | |
| </div> | |
| )} |
🤖 Prompt for AI Agents
In `@app/knowledge-base/page.tsx` around lines 986 - 1027, The delete-confirmation
modal rendered when showConfirmDelete is true needs ARIA roles and keyboard
handling: add role="dialog" and aria-modal="true" to the modal container, set an
initial focusable element (e.g., the Cancel or Delete button) via a ref,
implement an Escape key handler that calls setShowConfirmDelete(false) and
clears setCollectionToDelete(null), and implement a focus-trap that captures
Tab/Shift+Tab to cycle focus among modal focusable elements (use a useEffect in
the component that adds/removes a keydown listener which prevents focus leaving
the modal and restores focus on unmount). Update handlers like
handleConfirmDelete to still close the modal after action; apply the same
pattern later to the document picker and preview modals.
1 participant
This is how the UI looks like right now, please take a pull from this branch to try it out, also note that creation and deletion of knowledge base takes a little time to get updated in the UI, while the process already starts in the backend -
Summary
New Features
Improvements
Summary by CodeRabbit
Release Notes