Make Trends the default dashboard tab - #82
Conversation
The app leaderboard (Trends tab) is now the landing view, sorted by lowest error rate first. The "Latest Error Rate" column header is clickable to toggle between ascending and descending sort order. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughUpdated the "Trends" table in the export panel to support interactive sorting by latest error rate. Changed the default active tab from "By Kind" to "Trends," added sortable header styling with direction indicators, and implemented client-side click-driven sorting logic with state management. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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.
🧹 Nitpick comments (1)
src/commands/export.ts (1)
670-673: Consider using the delegated click handler for consistency.The event listener is re-attached every time
renderTrends()runs. While this works because the element is replaced viainnerHTML, the codebase pattern (lines 680-697) uses event delegation withdata-*attributes. This would be more consistent and slightly more efficient.♻️ Suggested refactor using delegated handler
Update the header to use a data attribute:
- var html = '<table><thead><tr><th>App</th><th>Direction</th><th>Sparkline (last 30d)</th><th class="sortable ' + sortClass + '" id="sort-error-rate">Latest Error Rate</th><th>Data Points</th></tr></thead><tbody>'; + var html = '<table><thead><tr><th>App</th><th>Direction</th><th>Sparkline (last 30d)</th><th class="sortable ' + sortClass + '" data-action="toggle-trends-sort">Latest Error Rate</th><th>Data Points</th></tr></thead><tbody>';Remove the inline listener and update the delegated handler at line 680:
document.addEventListener('click', function(e) { + // Trends sort toggle + var sortHeader = e.target.closest('[data-action="toggle-trends-sort"]'); + if (sortHeader && typeof window.toggleTrendsSort === 'function') { + window.toggleTrendsSort(); + return; + } // Copy handler var copyEl = e.target.closest('.copy[data-copy]');Then expose the toggle function:
- document.getElementById('sort-error-rate').addEventListener('click', function() { - sortAsc = !sortAsc; - renderTrends(); - }); } + + window.toggleTrendsSort = function() { + sortAsc = !sortAsc; + renderTrends(); + }; renderTrends();Based on learnings: "In HTML dashboard generation, NEVER use inline
onclickhandlers. Usedata-*attributes with the delegated click handler".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/export.ts` around lines 670 - 673, The direct event listener on the 'sort-error-rate' element should be replaced with a delegated click via the existing delegated handler used around renderTrends(); remove the document.getElementById('sort-error-rate').addEventListener(...) block, add a data-sort-error (e.g. data-sort-error="toggle") attribute to the header element that currently has id 'sort-error-rate', and update the delegated click handler (the function that currently handles other data-* clicks near renderTrends()) to check event.target.dataset.sortError and, when present, toggle the same sortAsc flag and call renderTrends(); alternatively extract the toggle into a small function toggleSortErrorRate() and call that from the delegated handler so the behavior is identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/commands/export.ts`:
- Around line 670-673: The direct event listener on the 'sort-error-rate'
element should be replaced with a delegated click via the existing delegated
handler used around renderTrends(); remove the
document.getElementById('sort-error-rate').addEventListener(...) block, add a
data-sort-error (e.g. data-sort-error="toggle") attribute to the header element
that currently has id 'sort-error-rate', and update the delegated click handler
(the function that currently handles other data-* clicks near renderTrends()) to
check event.target.dataset.sortError and, when present, toggle the same sortAsc
flag and call renderTrends(); alternatively extract the toggle into a small
function toggleSortErrorRate() and call that from the delegated handler so the
behavior is identical.
1 participant
Summary
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes