feat(frontend): improve devices empty state - #2923
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 43 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: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe devices table now provides a localized custom empty state with actions. ChangesDevices empty-state flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant DeviceTable
participant DataTable
participant DateRangePicker
DeviceTable->>DataTable: Render custom empty state
DeviceTable->>DateRangePicker: Open picker from empty-state action
DateRangePicker-->>DeviceTable: Restore focus on close
DeviceTable->>DataTable: Clear search and device filters
DataTable-->>DeviceTable: Reset pagination and reload
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Comment |
Merging this PR will not alter performance
Comparing Footnotes
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/components/tables/DeviceTable.vue`:
- Around line 604-634: Add the DaisyUI d-btn d-btn-link classes to the three
inline action buttons in the empty-state list—openDateRangePicker,
clearDeviceViewFilters, and refreshData—while preserving their existing inline
sizing and styling utilities.
🪄 Autofix
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: 4316422e-7189-42c5-8626-cd53c9e22620
📒 Files selected for processing (5)
messages/en.jsonplaywright/e2e/devices-empty-state.spec.tssrc/components/DataTable.vuesrc/components/DateRangePicker.vuesrc/components/tables/DeviceTable.vue
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Cap-go/capacitor-updater(manual)
There was a problem hiding this comment.
All reported issues were addressed
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Confidence score: 4/5
- In
src/components/DateRangePicker.vue, the empty-state “change time” control can only reopen (not close) the picker because clicks on thepositionAnchorare excluded from click-outside handling and the handler always callsopenPicker; users can get stuck with a picker that won’t toggle as expected, so the action should switch between open/close (or delegate to a true toggle handler).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/components/DateRangePicker.vue">
<violation number="1" location="src/components/DateRangePicker.vue:188">
P3: The empty-state 'change time' action can't toggle the picker closed: clicks on it are excluded from click-outside (positionAnchor) and its handler always calls openPicker, so clicking it again while open just re-syncs the draft and silently discards any selection made, unlike the toolbar trigger which toggles. Recommend making the invoker act as a toggle (expose/check isOpen) so a second click closes the picker.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| onClickOutside(popoverRef, (event) => { | ||
| const target = event.target as Node | null | ||
| if (target && triggerRef.value?.contains(target)) | ||
| if (target && (triggerRef.value?.contains(target) || positionAnchor.value?.contains(target))) |
There was a problem hiding this comment.
P3: The empty-state 'change time' action can't toggle the picker closed: clicks on it are excluded from click-outside (positionAnchor) and its handler always calls openPicker, so clicking it again while open just re-syncs the draft and silently discards any selection made, unlike the toolbar trigger which toggles. Recommend making the invoker act as a toggle (expose/check isOpen) so a second click closes the picker.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/components/DateRangePicker.vue, line 188:
<comment>The empty-state 'change time' action can't toggle the picker closed: clicks on it are excluded from click-outside (positionAnchor) and its handler always calls openPicker, so clicking it again while open just re-syncs the draft and silently discards any selection made, unlike the toolbar trigger which toggles. Recommend making the invoker act as a toggle (expose/check isOpen) so a second click closes the picker.</comment>
<file context>
@@ -182,7 +185,7 @@ defineExpose({ openPicker })
onClickOutside(popoverRef, (event) => {
const target = event.target as Node | null
- if (target && triggerRef.value?.contains(target))
+ if (target && (triggerRef.value?.contains(target) || positionAnchor.value?.contains(target)))
return
closePicker()
</file context>
There was a problem hiding this comment.
Fixed in 91d10a0. The exposed inline action now calls togglePicker(currentTarget) instead of always calling openPicker(), while the toolbar trigger calls the same toggle without passing its MouseEvent as an anchor. The focused Playwright test now verifies second-click close, Enter to reopen, and Escape focus restoration.
There was a problem hiding this comment.
@cubic-dev-ai please re-review this thread against 91d10a0 and resolve it if the toggle behavior is now addressed.
There was a problem hiding this comment.
@WcaleNieWolny I have started the AI code review. It will take a few minutes to complete.
There was a problem hiding this comment.
@cubic-dev-ai The requested toggle behavior remains fixed on the latest head a29a3a8 (implementation commit 91d10aebc): pressing Enter/Space on the active filter option clears it instead of reapplying it. Please re-review and resolve this thread if satisfied.
There was a problem hiding this comment.
@WcaleNieWolny I have started the AI code review. It will take a few minutes to complete.
There was a problem hiding this comment.
Correction to my previous reply: this thread concerns the empty-state date-picker toggle. The latest head exposes togglePicker() from DateRangePicker and DeviceTable calls it with the inline invoker, so a second mouse click closes the picker without resetting draft state. The focused Playwright test explicitly verifies open then close via the inline action. @cubic-dev-ai please re-review this implementation.
There was a problem hiding this comment.
@WcaleNieWolny I have started the AI code review. It will take a few minutes to complete.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@playwright/e2e/devices-empty-state.spec.ts`:
- Around line 64-69: Stabilize the request activity in the device empty-state
test before assigning requestCountBeforeRefresh: after the search assertion,
wait until the requests count remains unchanged to allow the page request or
debounced reload to complete. Then capture the baseline and preserve the
existing refresh click and +2 request-count assertion.
In `@src/components/tables/DeviceTable.vue`:
- Line 634: Update refreshData() to call cancelScheduledReload() as its first
action, ensuring any pending reloadTimer is cleared before starting the refresh
load and preventing the queued callback from superseding or duplicating the
request.
🪄 Autofix
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: cdab6daa-c611-4c47-8d3f-365089cf059c
📒 Files selected for processing (7)
messages/en.jsonplaywright/e2e/devices-empty-state.spec.tssrc/components/DataTable.vuesrc/components/DateRangePicker.vuesrc/components/admin/AdminFilterBar.vuesrc/components/tables/DeviceTable.vuetests/cli-app-permission-helper.test.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Cap-go/capacitor-updater(manual)
|



Summary
Test plan
bun lintbun typecheckbun test:unit(1,397 tests)CHOKIDAR_USEPOLLING=1 bun run buildSKIP_STRIPE_EMULATOR_START=1 bunx playwright test playwright/e2e/devices-empty-state.spec.ts --project=chromium(2 tests)Screenshots
Tested locally against the production-backed frontend.
Checklist
bun run lint:backend && bun run lint.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by CodeRabbit
New Features
Accessibility
Tests