feat(frontend): update vs observe log filters and clearer filter UI - #2930
Merged
Conversation
…ilter UI Separate Capgo update-process logs from observe events with quick filters, add clear-all, and replace the hard-to-notice ghost shortcuts with a proper filter modal and clickable buttons. Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
Contributor
📝 WalkthroughWalkthroughThe log filter dropdown becomes an accessible modal. The change adds keyboard and focus management, localized filter labels, observe/update shortcuts, action-filter classification, shared modal reuse, unit coverage, and visual-diff setup. ChangesLog filtering
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant TableLog
participant FilterModal
User->>TableLog: Click filter button
TableLog->>FilterModal: Open filter dialog
User->>FilterModal: Search or select filter shortcuts
FilterModal->>TableLog: Emit filter changes
User->>FilterModal: Click Done or press Escape
FilterModal->>TableLog: Emit close
TableLog->>User: Restore focus to filter button
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
Contributor
Merging this PR will not alter performance
Comparing 
Footnotes
|
… prep Make quick-filter shortcuts more button-like and open the logs filter modal during observe-logs visual diffs. Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
riderx
marked this pull request as ready for review
August 7, 2026 19:56
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_39249d87-8a92-4fae-aff5-dacd7e2b5d49) |
Contributor
Visual diff failedVisual diff did not produce a report. Check the workflow logs and artifacts. Commit: Open |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 13
🤖 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/visual-diff.config.ts`:
- Around line 48-51: Update the observe-logs prepare step to wrap the
filter-button click and modal wait in the same try/catch fallback pattern used
by the sibling route, allowing base-branch captures without the new selectors to
continue. Add the explicit timeout to the modal wait, matching that existing
route’s behavior.
In `@src/components/TableLog.vue`:
- Around line 434-442: Update the close button identified by
data-test="log-table-filters-close" and the shortcut buttons in the same filter
modal to use DaisyUI d-btn classes, matching the existing d-btn, d-btn-ghost,
and d-btn-primary styling used by the footer controls. Remove their hand-rolled
interactive Tailwind classes while preserving their labels, click handlers,
sizing, and behavior.
- Around line 56-116: Update closeFilterModal to restore focus only when
filterOpenButtonRef still resolves to a mounted, usable trigger, avoiding a
fallback to body when it has been unmounted. Also, when isFilterModalOpen is
true, apply inert to the application root and remove it when the modal closes,
while preserving the existing modal focus trap and Escape handling.
- Around line 528-530: Remove the form method="dialog" wrapper around the
backdrop button in the TableLog modal, leaving the button itself with its
existing type, aria-label, and closeFilterModal click handler unchanged.
- Around line 241-248: Extract the shared cleared-filters record construction
from clearAllFilters and applyFilterShortcut into a single helper, then call
that helper from both functions while preserving their existing filter-update
behavior.
- Around line 451-463: Add an aria-pressed binding to the shortcut buttons
rendered by the filterShortcuts loop, using isShortcutActive(shortcut) so
assistive technology receives the same active state represented by the existing
class binding.
- Around line 250-258: Update isShortcutActive to filter shortcut.filters to
keys present in props.filters before comparing lengths or membership, matching
applyFilterShortcut’s key filtering behavior. Use the filtered shortcut key set
for both the length check and every membership test, while preserving the
existing selected-filter logic.
- Line 389: Update the filter modal wrapper condition near the filter trigger to
depend on an unfiltered-filter availability check rather than filterList.length,
using a hasFilters computed property based on props.filters. Keep the modal
mounted when search results are empty so its no-results state remains reachable,
and reset filterSearchVal in closeFilterModal so reopening starts with the full
filter list.
- Around line 484-497: Update the filter option rendering in the filterList
v-for to use the filter name f instead of index i for both the Vue :key and the
generated input id/label :for suffix. Keep the checkbox name and filtering
behavior unchanged while ensuring each filter retains a stable DOM identity and
label association as the list changes.
- Line 482: Add the missing English `search` translation key in the locale
messages used by TableLog.vue, or replace the t('filter-options') usage with an
existing defined translation key. Ensure the modal displays translated text
instead of a raw key.
- Line 476: Replace the focus:outline-none utility in the TableLog control’s
class list with focus:outline-hidden, preserving the other classes and matching
the focus styling used by the component’s other controls.
In `@src/services/statsActions.ts`:
- Around line 136-170: Complete the Observe classification by adding ping and
uninstall to observeActions in src/services/statsActions.ts:136-170 before
deriving the exported groups. Update tests/stats-actions.unit.test.ts:32-36 by
adding both actions to OBSERVE_EXTRA_ACTIONS so the expected fixture matches
production behavior.
In `@tests/stats-actions.unit.test.ts`:
- Around line 80-84: Update the filter-key assertions in the test to build the
expected key set from statsActionFilters and compare it with the union of
observeActionFilterKeys and updateActionFilterKeys, rather than relying only on
combined lengths; also assert that the union size matches the expected set size
while preserving the existing disjointness check.
🪄 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: abdb0cbf-c2e5-4bae-8919-cd8afca55230
📒 Files selected for processing (6)
messages/en.jsonplaywright/visual-diff.config.tssrc/components/TableLog.vuesrc/components/tables/LogTable.vuesrc/services/statsActions.tstests/stats-actions.unit.test.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Cap-go/capacitor-updater(manual)
|
|
||
| <fieldset v-if="filterList.length" class="space-y-1"> | ||
| <legend class="mb-2 text-xs font-semibold uppercase tracking-wide text-slate-500 dark:text-slate-400"> | ||
| {{ t('filter-options') }} |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check that all translation keys used by the log filter modal exist in messages/en.json.
set -euo pipefail
file=$(fd -p 'messages/en.json' | head -n1)
echo "Locale file: $file"
for key in filter-options no-results clear-filters done close search \
filter-logs-modal-subtitle filter-shortcuts-label \
filter-shortcut-all-fail filter-shortcut-updates filter-shortcut-observe; do
if jq -e --arg k "$key" 'has($k)' "$file" >/dev/null; then
printf 'FOUND %s -> %s\n' "$key" "$(jq -r --arg k "$key" '.[$k]' "$file")"
else
printf 'MISSING %s\n' "$key"
fi
doneRepository: Cap-go/capgo.app
Length of output: 683
Add the missing search translation key.
messages/en.json does not define search, so the modal renders the raw key. Add the English translation or use an existing key.
🤖 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 `@src/components/TableLog.vue` at line 482, Add the missing English `search`
translation key in the locale messages used by TableLog.vue, or replace the
t('filter-options') usage with an existing defined translation key. Ensure the
modal displays translated text instead of a raw key.
Source: Learnings
Comment on lines
+136
to
+170
| /** Observe / health events (webview, crashes, launches, native version changes). */ | ||
| const observeActions = new Set<string>([ | ||
| 'app_moved_to_foreground', | ||
| 'app_moved_to_background', | ||
| 'app_launch_start', | ||
| 'app_launch_ready', | ||
| 'app_launch_timeout', | ||
| 'app_crash', | ||
| 'app_crash_native', | ||
| 'app_anr', | ||
| 'app_killed_low_memory', | ||
| 'app_killed_excessive_resource_usage', | ||
| 'app_initialization_failure', | ||
| 'app_memory_warning', | ||
| 'webview_javascript_error', | ||
| 'webview_unhandled_rejection', | ||
| 'webview_resource_error', | ||
| 'webview_security_policy_violation', | ||
| 'webview_unclean_restart', | ||
| 'webview_render_process_gone', | ||
| 'webview_content_process_terminated', | ||
| 'webview_dom_content_loaded', | ||
| 'webview_page_loaded', | ||
| 'os_version_changed', | ||
| 'native_app_version_changed', | ||
| ]) | ||
|
|
||
| export const observeActionFilterKeys = statsActionFilters | ||
| .filter(([, actionValue]) => observeActions.has(actionValue)) | ||
| .map(([filterKey]) => filterKey) | ||
|
|
||
| /** Live-update / OTA process events (everything that is not observe). */ | ||
| export const updateActionFilterKeys = statsActionFilters | ||
| .filter(([, actionValue]) => !observeActions.has(actionValue)) | ||
| .map(([filterKey]) => filterKey) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Keep the Observe classification and test contract complete.
updateActionFilterKeys is the complement of observeActions, so omitted health/lifecycle actions are silently exposed by the Update shortcut. Add ping and uninstall to the production set and its expected test fixture. The Capgo log documentation classifies these actions as a device heartbeat and an app-lifecycle event. (capgo.app)
src/services/statsActions.ts#L136-L170: AddpinganduninstalltoobserveActionsbefore deriving both exported groups.tests/stats-actions.unit.test.ts#L32-L36: AddpinganduninstalltoOBSERVE_EXTRA_ACTIONS.
📍 Affects 2 files
src/services/statsActions.ts#L136-L170(this comment)tests/stats-actions.unit.test.ts#L32-L36
🤖 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 `@src/services/statsActions.ts` around lines 136 - 170, Complete the Observe
classification by adding ping and uninstall to observeActions in
src/services/statsActions.ts:136-170 before deriving the exported groups. Update
tests/stats-actions.unit.test.ts:32-36 by adding both actions to
OBSERVE_EXTRA_ACTIONS so the expected fixture matches production behavior.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 6 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Share FilterModal between DataTable and TableLog to remove duplicated modal markup, and make observe-logs visual-diff prepare fall back on the legacy Actions dropdown for base captures. Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_8d0e87b0-2c06-423f-b0df-d37ba2c23928) |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/DataTable.vue`:
- Around line 594-609: Update the filter control IDs in
src/components/DataTable.vue lines 594-609 to use a per-instance prefix derived
from useId(), declared beside filterModalTitleId, for the checkbox id, label
for, and name. In src/components/TableLog.vue lines 405-431, apply the same
useId()-derived prefix beside filterModalTitleId to the log-filter-option label,
id, and name bindings, and also to the log-filter-search input id and associated
label for at lines 393-395.
In `@src/components/FilterModal.vue`:
- Around line 59-70: Update the open-state watcher around props.open to run
immediately on component initialization, so an initially open modal registers
onKeydown and focuses the first focusable element while preserving the existing
close cleanup and focus restoration behavior.
In `@src/components/TableLog.vue`:
- Line 401: Replace the focus:outline-none utility in the affected TableLog
control’s class list with focus:outline-hidden, matching the other controls in
the file and preserving the existing focus ring classes.
- Around line 65-67: Update closeFilterModal to reset filterSearchVal when
setting isFilterModalOpen.value to false, ensuring each modal opening starts
with an empty search term while preserving the existing close behavior.
- Around line 393-400: Add the missing "search" translation key to the English
messages configuration used by the t function, ensuring the existing
TableLog.vue label, aria-label, and placeholder resolve to the intended English
search text.
🪄 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: 09f0d0e0-d89f-4c1e-a8c4-96820fba5e0a
📒 Files selected for processing (6)
playwright/visual-diff.config.tssrc/components/DataTable.vuesrc/components/FilterModal.vuesrc/components/TableLog.vuesrc/composables/useFilterModal.tstests/stats-actions.unit.test.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Cap-go/capacitor-updater(manual)
Comment on lines
+594
to
+609
| <label | ||
| v-for="(f, i) in filterList" | ||
| :key="f" | ||
| :for="`filter-radio-example-${i}`" | ||
| class="flex min-h-11 cursor-pointer items-center rounded-md px-2 py-2 transition-colors duration-150 hover:bg-slate-50 dark:hover:bg-slate-800" | ||
| > | ||
| <div class="flex items-start justify-between gap-3 border-b border-slate-100 px-5 py-4 dark:border-slate-800"> | ||
| <div class="min-w-0"> | ||
| <h2 | ||
| :id="filterModalTitleId" | ||
| class="text-lg font-semibold leading-7 text-slate-950 dark:text-white" | ||
| > | ||
| {{ t(filterText ?? 'Filters') }} | ||
| </h2> | ||
| <p class="mt-1 text-sm leading-5 text-slate-600 dark:text-slate-300"> | ||
| {{ t('filter-modal-subtitle') }} | ||
| </p> | ||
| </div> | ||
| <button | ||
| type="button" | ||
| class="inline-flex h-11 w-11 shrink-0 items-center justify-center rounded-md text-slate-500 transition-colors duration-200 hover:bg-slate-100 hover:text-slate-900 focus:outline-hidden focus:ring-2 focus:ring-azure-500 dark:text-slate-300 dark:hover:bg-slate-800 dark:hover:text-white" | ||
| :aria-label="t('close')" | ||
| data-test="data-table-filters-close" | ||
| @click="closeFilterModal" | ||
| > | ||
| <IconClose class="h-5 w-5" /> | ||
| </button> | ||
| </div> | ||
|
|
||
| <div class="max-h-[min(28rem,60vh)] space-y-5 overflow-y-auto px-5 py-5"> | ||
| <div v-if="$slots['filter-extras']" class="space-y-4"> | ||
| <slot name="filter-extras" /> | ||
| </div> | ||
| <div | ||
| v-if="$slots['filter-extras'] && filterList.length" | ||
| class="border-t border-slate-200 dark:border-slate-700" | ||
| role="separator" | ||
| /> | ||
| <fieldset v-if="filterList.length" class="space-y-1"> | ||
| <legend class="mb-2 text-xs font-semibold uppercase tracking-wide text-slate-500 dark:text-slate-400"> | ||
| {{ t('filter-options') }} | ||
| </legend> | ||
| <label | ||
| v-for="(f, i) in filterList" | ||
| :key="i" | ||
| :for="`filter-radio-example-${i}`" | ||
| class="flex min-h-11 cursor-pointer items-center rounded-md px-2 py-2 transition-colors duration-150 hover:bg-slate-50 dark:hover:bg-slate-800" | ||
| > | ||
| <input | ||
| :id="`filter-radio-example-${i}`" | ||
| :checked="filters?.[f]" | ||
| type="checkbox" | ||
| :name="`filter-radio-${i}`" | ||
| class="h-4 w-4 shrink-0 rounded border-gray-300 text-azure-500 focus:ring-2 focus:ring-azure-500 dark:border-gray-600 dark:bg-gray-700 dark:ring-offset-gray-800" | ||
| @change=" | ||
| emit('update:filters', { ...filters, [f]: !filters?.[f] }) | ||
| " | ||
| > | ||
| <span class="ml-3 min-w-0 text-sm font-medium text-slate-900 dark:text-slate-200"> | ||
| {{ getFilterLabel(f) }} | ||
| </span> | ||
| </label> | ||
| </fieldset> | ||
| </div> | ||
|
|
||
| <div class="flex flex-col-reverse gap-2 border-t border-slate-100 px-5 py-4 sm:flex-row sm:items-center sm:justify-between dark:border-slate-800"> | ||
| <button | ||
| type="button" | ||
| class="d-btn d-btn-ghost min-h-11" | ||
| data-test="data-table-filters-clear" | ||
| :disabled="!filterActivated" | ||
| @click="clearAllFilters" | ||
| > | ||
| {{ t('clear-filters') }} | ||
| </button> | ||
| <button | ||
| type="button" | ||
| class="d-btn d-btn-primary min-h-11" | ||
| data-test="data-table-filters-done" | ||
| @click="closeFilterModal" | ||
| > | ||
| {{ t('done') }} | ||
| </button> | ||
| </div> | ||
| </div> | ||
| <form method="dialog" class="d-modal-backdrop"> | ||
| <button type="button" :aria-label="t('close')" @click="closeFilterModal" /> | ||
| </form> | ||
| </div> | ||
| </Teleport> | ||
| <input | ||
| :id="`filter-radio-example-${i}`" | ||
| :checked="filters?.[f]" | ||
| type="checkbox" | ||
| :name="`filter-radio-${i}`" | ||
| class="h-4 w-4 shrink-0 rounded border-gray-300 text-azure-500 focus:ring-2 focus:ring-azure-500 dark:border-gray-600 dark:bg-gray-700 dark:ring-offset-gray-800" | ||
| @change=" | ||
| emit('update:filters', { ...filters, [f]: !filters?.[f] }) | ||
| " | ||
| > |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Filter modal form controls build IDs from literal prefixes instead of a per-instance prefix. Both components already derive the dialog title ID from useId(), but the filter form controls use fixed literals. If two instances of the same table component render on one page, the IDs collide and each <label for> binds to the first matching input, so clicking a label toggles the wrong control.
src/components/DataTable.vue#L594-L609: replace thefilter-radio-example-${i}andfilter-radio-${i}literals with a prefix derived fromuseId(), declared next tofilterModalTitleId.src/components/TableLog.vue#L405-L431: replace thelog-filter-option-${f}literal in the:for,:id, and:namebindings with a prefix derived fromuseId(), declared next tofilterModalTitleId; apply the same prefix to thelog-filter-searchinput ID at Line 395 and itsforat Line 393.
Based on coding guidelines "reusable components must generate stable IDs and accessible names by default".
📍 Affects 2 files
src/components/DataTable.vue#L594-L609(this comment)src/components/TableLog.vue#L405-L431
🤖 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 `@src/components/DataTable.vue` around lines 594 - 609, Update the filter
control IDs in src/components/DataTable.vue lines 594-609 to use a per-instance
prefix derived from useId(), declared beside filterModalTitleId, for the
checkbox id, label for, and name. In src/components/TableLog.vue lines 405-431,
apply the same useId()-derived prefix beside filterModalTitleId to the
log-filter-option label, id, and name bindings, and also to the
log-filter-search input id and associated label for at lines 393-395.
Source: Coding guidelines
Comment on lines
+59
to
+70
| watch(() => props.open, async (open) => { | ||
| if (open) { | ||
| window.addEventListener('keydown', onKeydown) | ||
| await nextTick() | ||
| getFocusable()[0]?.focus() | ||
| } | ||
| else { | ||
| window.removeEventListener('keydown', onKeydown) | ||
| await nextTick() | ||
| props.restoreFocusEl?.focus() | ||
| } | ||
| }) |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
Handle the case where the modal mounts with open already true.
The watcher is not immediate. If a parent renders FilterModal with :open="true" on first render, the component registers no keydown listener and sets no initial focus. Escape and Tab trapping then do not work. The two call sites in this PR start closed, so the gap is latent. Make the behavior explicit for future consumers.
♻️ Proposed refactor
-watch(() => props.open, async (open) => {
+watch(() => props.open, async (open, previous) => {
if (open) {
window.addEventListener('keydown', onKeydown)
await nextTick()
getFocusable()[0]?.focus()
}
- else {
+ else if (previous) {
window.removeEventListener('keydown', onKeydown)
await nextTick()
props.restoreFocusEl?.focus()
}
-})
+}, { immediate: true })📝 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.
Suggested change
| watch(() => props.open, async (open) => { | |
| if (open) { | |
| window.addEventListener('keydown', onKeydown) | |
| await nextTick() | |
| getFocusable()[0]?.focus() | |
| } | |
| else { | |
| window.removeEventListener('keydown', onKeydown) | |
| await nextTick() | |
| props.restoreFocusEl?.focus() | |
| } | |
| }) | |
| watch(() => props.open, async (open, previous) => { | |
| if (open) { | |
| window.addEventListener('keydown', onKeydown) | |
| await nextTick() | |
| getFocusable()[0]?.focus() | |
| } | |
| else if (previous) { | |
| window.removeEventListener('keydown', onKeydown) | |
| await nextTick() | |
| props.restoreFocusEl?.focus() | |
| } | |
| }, { immediate: true }) |
🤖 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 `@src/components/FilterModal.vue` around lines 59 - 70, Update the open-state
watcher around props.open to run immediately on component initialization, so an
initially open modal registers onKeydown and focuses the first focusable element
while preserving the existing close cleanup and focus restoration behavior.
Comment on lines
+65
to
67
| function closeFilterModal() { | ||
| isFilterModalOpen.value = false | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reset filterSearchVal when the modal closes.
closeFilterModal keeps the previous search term. When the user reopens the modal, the option list is still narrowed by the old term. The shortcut buttons stay visible, so the state is recoverable, but the cause is not obvious. This point was raised in an earlier review and is still open.
♻️ Proposed fix
function closeFilterModal() {
isFilterModalOpen.value = false
+ filterSearchVal.value = ''
}🤖 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 `@src/components/TableLog.vue` around lines 65 - 67, Update closeFilterModal to
reset filterSearchVal when setting isFilterModalOpen.value to false, ensuring
each modal opening starts with an empty search term while preserving the
existing close behavior.
Comment on lines
+393
to
400
| <label for="log-filter-search" class="sr-only">{{ t('search') }}</label> | ||
| <input | ||
| id="log-filter-search" | ||
| v-model="filterSearchVal" | ||
| type="text" | ||
| name="log-filter-search" | ||
| :aria-label="t('search')" | ||
| :placeholder="t('search')" |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check the translation keys used by the log filter modal.
set -euo pipefail
file=$(fd -p 'messages/en.json' | head -n1)
echo "Locale file: $file"
for key in search filter-options no-results clear-filters done close \
filter-logs-modal-subtitle filter-modal-subtitle filter-shortcuts-label; do
if jq -e --arg k "$key" 'has($k)' "$file" >/dev/null; then
printf 'FOUND %s -> %s\n' "$key" "$(jq -r --arg k "$key" '.[$k]' "$file")"
else
printf 'MISSING %s\n' "$key"
fi
done
# Surface every translation key referenced by the new modal components.
rg -n --type=vue -o "t\('([a-z0-9-]+)'\)" -r '$1' src/components/FilterModal.vue src/components/TableLog.vue | sort -uRepository: Cap-go/capgo.app
Length of output: 1667
🏁 Script executed:
set -euo pipefail
printf '%s\n' 'Translation key definitions:'
rg -n --glob 'messages/*.json' '"search"|"filter-options"|"no-results"' messages || true
printf '%s\n' 'Relevant component context:'
sed -n '228,250p;386,405p' src/components/TableLog.vue
printf '%s\n' 'Locale and fallback configuration:'
rg -n --glob '*.{ts,js,vue}' 'createI18n|fallbackLocale|messages/' src | head -80 || trueRepository: Cap-go/capgo.app
Length of output: 2184
Add the missing search translation key to messages/en.json. The input uses it for its label, accessible name, and placeholder.
🤖 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 `@src/components/TableLog.vue` around lines 393 - 400, Add the missing "search"
translation key to the English messages configuration used by the t function,
ensuring the existing TableLog.vue label, aria-label, and placeholder resolve to
the intended English search text.
| :placeholder="t('search')" | ||
| class="w-full px-3 py-2 mb-2 text-sm border border-gray-300 rounded-md dark:border-gray-600 dark:bg-gray-700 dark:text-white focus:outline-none focus:ring-2 focus:ring-blue-500" | ||
| @click.stop | ||
| class="w-full rounded-md border border-slate-300 px-3 py-2.5 text-sm text-slate-900 focus:border-azure-500 focus:outline-none focus:ring-2 focus:ring-azure-500 dark:border-slate-600 dark:bg-slate-800 dark:text-white" |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use focus:outline-hidden instead of focus:outline-none.
In Tailwind CSS v4, outline-none sets outline-style: none. That removes the focus indicator in forced-colors mode, and the focus:ring-2 utility is not rendered there either. outline-hidden keeps a transparent 2px outline, which forced-colors mode paints. Every other control in this file already uses focus:outline-hidden. This point was raised in an earlier review and is still open.
♿ Proposed fix
- class="w-full rounded-md border border-slate-300 px-3 py-2.5 text-sm text-slate-900 focus:border-azure-500 focus:outline-none focus:ring-2 focus:ring-azure-500 dark:border-slate-600 dark:bg-slate-800 dark:text-white"
+ class="w-full rounded-md border border-slate-300 px-3 py-2.5 text-sm text-slate-900 focus:border-azure-500 focus:outline-hidden focus:ring-2 focus:ring-azure-500 dark:border-slate-600 dark:bg-slate-800 dark:text-white"🤖 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 `@src/components/TableLog.vue` at line 401, Replace the focus:outline-none
utility in the affected TableLog control’s class list with focus:outline-hidden,
matching the other controls in the file and preserving the existing focus ring
classes.
|
There was a problem hiding this comment.
2 issues found across 6 files (changes from recent commits).
Confidence score: 4/5
- In
src/components/FilterModal.vue, the non-immediate watcher means an initially open modal skips dialog setup on mount, so Escape handling, focus trap, and initial focus can fail for users right away—initialize from the initialopenvalue (or make the watcher immediate) to de-risk accessibility and keyboard behavior. - In
src/composables/useFilterModal.ts, the composable-style filename doesn’t match the current plain helper export (createClearedFilters()), which can mislead future usage and maintenance—either rename the file to a utility-style name or align the implementation with composable conventions.
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/composables/useFilterModal.ts">
<violation number="1" location="src/composables/useFilterModal.ts:1">
P3: The file name `useFilterModal.ts` follows the composable convention, but it only exports a plain helper `createClearedFilters()` with no reactive state. Consider renaming it (e.g. filterUtils.ts) or matching the actual module contents, since the `use*` name misleads readers into expecting composable behavior and doesn't represent a `useXxx` composable.</violation>
</file>
<file name="src/components/FilterModal.vue">
<violation number="1" location="src/components/FilterModal.vue:59">
P2: An initially-open `FilterModal` is not initialized as a dialog because this non-immediate watcher never runs on mount, leaving Escape handling, focus trapping, and initial focus inactive. Handle the initial `open` value while avoiding focus restoration for the initial `false` state.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| } | ||
| } | ||
|
|
||
| watch(() => props.open, async (open) => { |
There was a problem hiding this comment.
P2: An initially-open FilterModal is not initialized as a dialog because this non-immediate watcher never runs on mount, leaving Escape handling, focus trapping, and initial focus inactive. Handle the initial open value while avoiding focus restoration for the initial false state.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/components/FilterModal.vue, line 59:
<comment>An initially-open `FilterModal` is not initialized as a dialog because this non-immediate watcher never runs on mount, leaving Escape handling, focus trapping, and initial focus inactive. Handle the initial `open` value while avoiding focus restoration for the initial `false` state.</comment>
<file context>
@@ -0,0 +1,146 @@
+ }
+}
+
+watch(() => props.open, async (open) => {
+ if (open) {
+ window.addEventListener('keydown', onKeydown)
</file context>
| @@ -0,0 +1,5 @@ | |||
| export function createClearedFilters(filters: Record<string, boolean> | undefined): Record<string, boolean> { | |||
There was a problem hiding this comment.
P3: The file name useFilterModal.ts follows the composable convention, but it only exports a plain helper createClearedFilters() with no reactive state. Consider renaming it (e.g. filterUtils.ts) or matching the actual module contents, since the use* name misleads readers into expecting composable behavior and doesn't represent a useXxx composable.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/composables/useFilterModal.ts, line 1:
<comment>The file name `useFilterModal.ts` follows the composable convention, but it only exports a plain helper `createClearedFilters()` with no reactive state. Consider renaming it (e.g. filterUtils.ts) or matching the actual module contents, since the `use*` name misleads readers into expecting composable behavior and doesn't represent a `useXxx` composable.</comment>
<file context>
@@ -0,0 +1,5 @@
+export function createClearedFilters(filters: Record<string, boolean> | undefined): Record<string, boolean> {
+ return Object.fromEntries(
+ Object.keys(filters ?? {}).map(key => [key, false]),
</file context>
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
2 participants
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 (AI generated)
FilterModal(used by logs + DataTable) with bordered shortcut buttons, search, checkboxes, and Done/Clearobserve-logsprepare now falls back on the legacy Actions dropdown for base captures; Sonar duplication reduced by extracting shared modalMotivation (AI generated)
Logs currently mix Capgo update-process events with observe/health events. Users need a fast way to isolate either set. The previous “All failures” shortcut also looked like plain text, so people did not click it.
Business Impact (AI generated)
Makes observe and update debugging faster in the console, reducing support friction when customers investigate OTA failures vs app/webview health.
Visual changes (AI generated)
Before
Logs filter before
After
Logs filter after
After with Update process selected
Logs filter after Update process selected
Test Plan (AI generated)
/app/<app>/observe/logsand open the Actions filter modalbunx vitest run tests/stats-actions.unit.test.tsobserve-logscapture succeeds on base + headGenerated with AI
To show artifacts inline, enable in settings.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by CodeRabbit
New Features
Documentation
Tests