Feat/dashboard url management phase 3 4 - #30
Merged
Conversation
|
Warning Review limit reached
Next review available in: 47 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (23)
📝 WalkthroughWalkthroughThe dashboard now includes reusable UI primitives, complete URL creation and bulk creation flows, searchable and filterable URL management, detailed analytics with visits, charts, QR codes, export, and deletion, plus agent configuration and phase specification updates. ChangesDashboard UI foundation
URL creation and management
Project configuration and specification
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant UrlsListPage
participant URL API
participant Dialog
User->>UrlsListPage: apply filters or open row action
UrlsListPage->>URL API: fetch filtered URL data
URL API-->>UrlsListPage: paginated URLs and metadata
User->>Dialog: confirm deletion
Dialog->>UrlsListPage: confirm action
UrlsListPage->>URL API: soft-delete URL
URL API-->>UrlsListPage: deletion result
UrlsListPage->>URL API: refresh URL list
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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: 17
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (10)
apps/dashboard/src/pages/UrlDetailPage.tsx-97-99 (1)
97-99: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winBind the download format to the generated QR result.
After generating PNG, changing the selector to SVG leaves the PNG blob displayed but downloads it with an
.svgextension. Store{ url, format }together and revoke replaced object URLs during cleanup.Also applies to: 168-175, 582-589, 611-620
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/dashboard/src/pages/UrlDetailPage.tsx` around lines 97 - 99, Update the QR result state and generation flow around qrDataUrl, qrFormat, and setQrDataUrl to store the generated object URL together with its actual format, and use that stored format when naming downloads instead of the current selector value. Revoke replaced object URLs when updating the result and revoke the final URL during component cleanup, while preserving the existing display behavior.apps/dashboard/src/pages/UrlDetailPage.tsx-580-598 (1)
580-598: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAssociate the QR labels with their controls.
Add matching
htmlFor/idvalues so assistive technology announces the Format and Logo URL labels.Proposed fix
-<label className="text-xs font-medium text-muted-foreground">Format</label> +<label htmlFor="qr-format" className="text-xs font-medium text-muted-foreground">Format</label> <select + id="qr-format" value={qrFormat} -<label className="text-xs font-medium text-muted-foreground">Logo URL (optional)</label> +<label htmlFor="qr-logo" className="text-xs font-medium text-muted-foreground">Logo URL (optional)</label> <input + id="qr-logo" value={qrLogo}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/dashboard/src/pages/UrlDetailPage.tsx` around lines 580 - 598, Associate the Format and Logo URL labels with their controls by adding matching htmlFor attributes and unique id attributes to the corresponding select and input elements in the QR settings form. Update only the controls surrounding qrFormat and qrLogo, preserving their existing behavior.apps/dashboard/src/components/copy-button.tsx-16-30 (1)
16-30: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winHandle fallback copy failures
document.execCommand("copy")can returnfalse, but this branch still marks the copy as successful. Wrap the temporary input cleanup infinallyand only setcopiedwhen the command actually succeeds.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/dashboard/src/components/copy-button.tsx` around lines 16 - 30, Update handleCopy’s fallback branch to check the boolean result of document.execCommand("copy") before calling setCopied(true) and scheduling the reset. Ensure the temporary input is always removed with cleanup in a finally block, including when the fallback command fails or throws.apps/dashboard/src/components/ui/select.tsx-7-20 (1)
7-20: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMerge caller styles without dropping the select arrow styles. Move
styleahead of{...props}and merge it into the existing inline styles; otherwise any caller-suppliedstylewill replacebackgroundImage, positioning, andpaddingRight.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/dashboard/src/components/ui/select.tsx` around lines 7 - 20, Update the select component’s inline style handling in the forwarded-ref render function: destructure the caller’s style from props or otherwise place it before the prop spread, then merge it with the existing arrow styles so caller properties are preserved without replacing backgroundImage, backgroundPosition, or paddingRight.apps/dashboard/src/pages/UrlDetailPage.tsx-230-235 (1)
230-235: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAvoid nesting
<Button>inside<Link>
Buttonrenders a native<button>, so this pattern creates nested interactive elements. Use the link itself styled like a button, or addasChildsupport before composing it this way.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/dashboard/src/pages/UrlDetailPage.tsx` around lines 230 - 235, Update the Settings navigation in UrlDetailPage to avoid wrapping the Button component in a Link. Use the link itself with button styling, or use Button’s asChild support if available, while preserving the existing `/urls/${code}/settings` destination, outline variant, small size, icon, and label.apps/dashboard/src/components/ui/pagination.tsx-73-85 (1)
73-85: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winGive the page-size selector an accessible name.
The adjacent “Show” text is not associated with the
<select>. Wrap both in a<label>or addaria-label="Results per page".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/dashboard/src/components/ui/pagination.tsx` around lines 73 - 85, Update the page-size selector in the pagination component so it has an accessible name, either by associating the adjacent “Show” text through a label or by adding the specified aria-label to the select. Preserve the existing pageSize value and onPageSizeChange behavior.apps/dashboard/src/components/ui/tooltip.tsx-23-39 (1)
23-39: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winExpose tooltips on keyboard focus.
Show and hide the tooltip on
focus/bluras well as hover, addrole="tooltip", and connect it to the trigger witharia-describedby. The current truncated-URL tooltip is mouse-only.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/dashboard/src/components/ui/tooltip.tsx` around lines 23 - 39, Update the tooltip component’s trigger container and conditional tooltip element to support keyboard accessibility: track focus alongside mouse enter/leave in the existing visibility handlers, add aria-describedby to the trigger referencing a stable tooltip id, and assign role="tooltip" to the rendered content. Preserve the current hover behavior and content rendering.apps/dashboard/src/pages/UrlsListPage.tsx-345-351 (1)
345-351: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winLabel the selection checkboxes.
Add an accessible name such as “Select all URLs” and
Select ${url.code}. Otherwise screen readers announce multiple indistinguishable checkboxes.Also applies to: 382-388
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/dashboard/src/pages/UrlsListPage.tsx` around lines 345 - 351, Add accessible labels to the selection checkboxes in the URLs table: label the header checkbox in the select-all control as “Select all URLs,” and label each row checkbox using the corresponding URL code, such as “Select ${url.code}.” Update both the header checkbox and row checkbox rendering near the selection controls without changing their selection behavior.apps/dashboard/src/pages/CreateUrlPage.tsx-105-107 (1)
105-107: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winEnforce the documented custom-code character set.
Values containing spaces or unsupported punctuation pass client validation despite the UI promising only letters, numbers, hyphens, and underscores.
Proposed fix
- if (form.customCode && (form.customCode.length < 3 || form.customCode.length > 16)) { - errs.customCode = "Custom code must be 3-16 characters" + if ( + form.customCode && + (form.customCode.length < 3 || + form.customCode.length > 16 || + !/^[A-Za-z0-9_-]+$/.test(form.customCode)) + ) { + errs.customCode = "Custom code must be 3-16 letters, numbers, hyphens, or underscores" }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/dashboard/src/pages/CreateUrlPage.tsx` around lines 105 - 107, Update the customCode validation in CreateUrlPage to reject any value containing characters outside letters, numbers, hyphens, and underscores, while preserving the existing 3–16 character length checks and error behavior.apps/dashboard/src/components/ui/switch.tsx-8-43 (1)
8-43: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep the switch visuals in sync for uncontrolled usage.
SwitchPropsstill acceptsdefaultChecked, but the label/thumb classes are driven only bychecked. If a caller uses this uncontrolled, the input toggles while the UI stays in the off state. Either track internal checked state or removedefaultCheckedfrom the public props.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/dashboard/src/components/ui/switch.tsx` around lines 8 - 43, The Switch component’s visual state must update when used with defaultChecked without a controlled checked prop. Track the input’s internal checked state in the Switch component, initialize it from checked or defaultChecked, update it in handleChange, and use that state for the label and thumb classes while preserving controlled checked behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/dashboard/src/components/ui/command.tsx`:
- Around line 10-23: Update the Command component to make filter, search,
setSearch, and inputRef available to CommandInput, CommandList, and CommandItem
through the existing child communication pattern, so filtering operates
correctly. Stabilize the input focus behavior by preserving one ref across
renders and using it for focus, or remove the unsupported ref API and rely on
autoFocus.
In `@apps/dashboard/src/components/ui/dialog.tsx`:
- Around line 28-41: Update the dialog component around the overlay and inner
element with role="dialog" to add aria-modal and title/description associations
using the dialog’s title and description elements. On open, move focus to an
appropriate element inside the modal; while open, intercept Tab navigation to
cycle focus only among modal descendants, including correct forward and reverse
wrapping. Track the previously focused trigger and restore focus to it when
onClose completes, while preserving the existing overlay-click behavior.
In `@apps/dashboard/src/components/ui/pagination.tsx`:
- Around line 15-22: Replace the O(totalPages) loop that builds pages with a
fixed candidate set containing page 1, totalPages, and the valid neighboring
pages around page, removing duplicates and out-of-range values. Sort the
candidates, then insert "..." between non-consecutive page numbers so the
rendered output remains unchanged without iterating through every page.
In `@apps/dashboard/src/components/ui/popover.tsx`:
- Around line 10-29: Update Popover to provide its open state and setter through
context, then have PopoverTrigger consume that context and toggle the state on
activation. Have PopoverContent consume the same context and render its children
only while open, while preserving controlled and uncontrolled behavior in
Popover and its outside-click handling.
In `@apps/dashboard/src/components/ui/tabs.tsx`:
- Around line 44-48: Implement the complete accessibility behavior across
TabsList, TabsTrigger, and TabsContent: add the tablist, tab, and tabpanel
roles; wire each trigger to its panel with stable matching ids and
aria-controls/aria-labelledby; expose selection with aria-selected; and
implement roving focus with arrow-key navigation while preserving existing tab
behavior. Alternatively, replace these components with a proven accessible tabs
primitive and retain the current API.
In `@apps/dashboard/src/pages/BulkCreatePage.tsx`:
- Around line 180-203: Update the result-row rendering in BulkCreatePage to
display the corresponding original input URL for every result, using
result.index and falling back to the response position when unavailable. Keep
the existing success and failure content, but add the URL beside it so failed
rows can be identified.
In `@apps/dashboard/src/pages/CreateUrlPage.tsx`:
- Around line 309-319: Associate the “Block bots” label with the nested Switch
in the CreateUrlPage block-bots control by adding a matching id and htmlFor, or
provide an aria-label of “Block bots” directly on the Switch.
- Around line 16-22: Update the TTL_PRESETS and associated form
validation/submission logic so the “Never” option, represented by 0, is accepted
consistently with the API contract. Ensure selecting “Never” no longer fails the
minimum-TTL validation while preserving validation for other invalid values.
- Around line 95-103: Centralize strict HTTP(S) normalization in a shared
helper, then update validation and submission flows to reuse its normalized
result. In apps/dashboard/src/pages/CreateUrlPage.tsx lines 95-103, validate
through the helper and allow only http: or https:; in lines 134-135, submit that
normalized value. In apps/dashboard/src/pages/BulkCreatePage.tsx lines 45-52,
normalize each input line with the same helper, and in lines 71-72 send the
previously normalized values without reapplying separate URL rules.
- Around line 127-132: Update the activeAt construction in the form submission
flow to interpret form.activeAt and form.activeTime as local values, create a
local Date from them, and serialize the result with toISOString(). Preserve the
existing midnight fallback when form.activeTime is absent, and avoid appending Z
directly to the local control values.
In `@apps/dashboard/src/pages/UrlDetailPage.tsx`:
- Around line 116-134: Protect the analytics requests in fetchVisits,
fetchStats, and exportVisitsCsv by enforcing the authenticated session token and
backend ownership validation for the short code. Update the corresponding
endpoint handling so unauthorized users cannot access visits, stats, or CSV
exports, while preserving the existing success and error flows for authorized
requests.
- Around line 104-147: Update fetchUrlInfo, fetchVisits, fetchStats, and the
useEffect request flow to use an AbortController per code change, pass its
signal through the corresponding getUrlInfo/getUrlVisits/getUrlStats API calls,
and abort the previous controller during cleanup. Ignore AbortError results and
gate state updates, toasts, navigation, and loading changes to the active
request so stale responses cannot affect the current URL.
In `@apps/dashboard/src/pages/UrlsListPage.tsx`:
- Around line 95-122: Update fetchData and its useEffect to prevent superseded
requests from updating state: use an AbortController or request-generation guard
so only the latest filter request may apply setUrls, setPagination, setTags, and
setCollections. Ensure canceled or stale requests do not trigger toast errors,
loading state is finalized only for the current request, and clean up the
previous request when dependencies change.
- Around line 431-452: The row menu in the URL list creates nested interactive
elements: `DropdownMenuTrigger` wraps a button, and the Settings `Link` wraps a
button-based `DropdownMenuItem`. Update the `DropdownMenuTrigger`, Settings
`Link`, and `DropdownMenuItem` composition so each menu action renders exactly
one final interactive element, using child-slot composition or the primitives’
`asChild` behavior without changing the existing actions.
- Around line 103-111: Update the data-loading flow in UrlsListPage around
listTags and listCollections so all tag and collection options are available
before setTags and setCollections build the filter menus. Page through both
endpoints until all pagination results are collected, preserving the existing
empty-fallback behavior, or replace the loading with server-backed filter
search.
In `@opencode.json`:
- Around line 450-452: Remove the hardcoded x-api-key value from the MCP
configuration block in opencode.json and source it through an
environment-variable or secure secret-management reference instead. Preserve the
remote URL and enabled setting while ensuring the headers configuration resolves
the key at runtime without committing the secret.
- Around line 80-81: Remove the live x-api-key value from the opencode
configuration and load it through an environment variable or secret-store
reference instead. If the subagent model removal near the edit/bash settings is
intentional, update the corresponding model schema documentation in the README
and migration guide to match.
---
Minor comments:
In `@apps/dashboard/src/components/copy-button.tsx`:
- Around line 16-30: Update handleCopy’s fallback branch to check the boolean
result of document.execCommand("copy") before calling setCopied(true) and
scheduling the reset. Ensure the temporary input is always removed with cleanup
in a finally block, including when the fallback command fails or throws.
In `@apps/dashboard/src/components/ui/pagination.tsx`:
- Around line 73-85: Update the page-size selector in the pagination component
so it has an accessible name, either by associating the adjacent “Show” text
through a label or by adding the specified aria-label to the select. Preserve
the existing pageSize value and onPageSizeChange behavior.
In `@apps/dashboard/src/components/ui/select.tsx`:
- Around line 7-20: Update the select component’s inline style handling in the
forwarded-ref render function: destructure the caller’s style from props or
otherwise place it before the prop spread, then merge it with the existing arrow
styles so caller properties are preserved without replacing backgroundImage,
backgroundPosition, or paddingRight.
In `@apps/dashboard/src/components/ui/switch.tsx`:
- Around line 8-43: The Switch component’s visual state must update when used
with defaultChecked without a controlled checked prop. Track the input’s
internal checked state in the Switch component, initialize it from checked or
defaultChecked, update it in handleChange, and use that state for the label and
thumb classes while preserving controlled checked behavior.
In `@apps/dashboard/src/components/ui/tooltip.tsx`:
- Around line 23-39: Update the tooltip component’s trigger container and
conditional tooltip element to support keyboard accessibility: track focus
alongside mouse enter/leave in the existing visibility handlers, add
aria-describedby to the trigger referencing a stable tooltip id, and assign
role="tooltip" to the rendered content. Preserve the current hover behavior and
content rendering.
In `@apps/dashboard/src/pages/CreateUrlPage.tsx`:
- Around line 105-107: Update the customCode validation in CreateUrlPage to
reject any value containing characters outside letters, numbers, hyphens, and
underscores, while preserving the existing 3–16 character length checks and
error behavior.
In `@apps/dashboard/src/pages/UrlDetailPage.tsx`:
- Around line 97-99: Update the QR result state and generation flow around
qrDataUrl, qrFormat, and setQrDataUrl to store the generated object URL together
with its actual format, and use that stored format when naming downloads instead
of the current selector value. Revoke replaced object URLs when updating the
result and revoke the final URL during component cleanup, while preserving the
existing display behavior.
- Around line 580-598: Associate the Format and Logo URL labels with their
controls by adding matching htmlFor attributes and unique id attributes to the
corresponding select and input elements in the QR settings form. Update only the
controls surrounding qrFormat and qrLogo, preserving their existing behavior.
- Around line 230-235: Update the Settings navigation in UrlDetailPage to avoid
wrapping the Button component in a Link. Use the link itself with button
styling, or use Button’s asChild support if available, while preserving the
existing `/urls/${code}/settings` destination, outline variant, small size,
icon, and label.
In `@apps/dashboard/src/pages/UrlsListPage.tsx`:
- Around line 345-351: Add accessible labels to the selection checkboxes in the
URLs table: label the header checkbox in the select-all control as “Select all
URLs,” and label each row checkbox using the corresponding URL code, such as
“Select ${url.code}.” Update both the header checkbox and row checkbox rendering
near the selection controls without changing their selection behavior.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: db0784b8-800d-44bc-ad06-ad4d4ec0d34b
📒 Files selected for processing (19)
apps/dashboard/src/components/copy-button.tsxapps/dashboard/src/components/status-badge.tsxapps/dashboard/src/components/ui/command.tsxapps/dashboard/src/components/ui/dialog.tsxapps/dashboard/src/components/ui/label.tsxapps/dashboard/src/components/ui/pagination.tsxapps/dashboard/src/components/ui/popover.tsxapps/dashboard/src/components/ui/select.tsxapps/dashboard/src/components/ui/switch.tsxapps/dashboard/src/components/ui/table.tsxapps/dashboard/src/components/ui/tabs.tsxapps/dashboard/src/components/ui/textarea.tsxapps/dashboard/src/components/ui/tooltip.tsxapps/dashboard/src/pages/BulkCreatePage.tsxapps/dashboard/src/pages/CreateUrlPage.tsxapps/dashboard/src/pages/UrlDetailPage.tsxapps/dashboard/src/pages/UrlsListPage.tsxopencode.jsonspecs/03-third-fourth-phase.md
…, and component hardening
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit