ui: redesign error inspector header and summary bar with minimal layo… - #12
Conversation
…ut and pulsing status indicators
|
Warning Review limit reached
Next review available in: 48 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: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughRefactors the ErrorInspectorPage UI: replaces the summary-card header and chip-style platform counts with a unified info bar, adds animated widgets (pulsing dot, count-up, animated empty state), and rewrites the platform filter chip with tap-pop animation while preserving existing filter logic. ChangesError Inspector UI Redesign
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant PlatformFilterChip
participant ErrorFilterProvider
participant InfoBar
participant ErrorInspectorPage
User->>PlatformFilterChip: tap chip
PlatformFilterChip->>PlatformFilterChip: play tap-pop animation
PlatformFilterChip->>ErrorFilterProvider: update filter state
ErrorFilterProvider-->>ErrorInspectorPage: notify state change
ErrorInspectorPage->>InfoBar: rebuild with filtered counts
InfoBar->>InfoBar: animate _CountUp for new totals
InfoBar-->>User: render updated bar/header
Estimated code review effort: 3 (Moderate) | ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the error inspector page by replacing the summary cards with a unified info bar, introducing new custom widgets like _PulsingDot, _CountUp, and _EmptyStateWithPulse, and adding hover and tap animations to the platform filter chips. The review feedback highlights a few issues: a non-functional tap animation in _PlatformFilterChip due to static scale values and unused state variables, a duplicate vertical divider rendered in the unified info bar, and a performance optimization opportunity in _PulsingDot to animate color alpha directly instead of using an Opacity widget.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| class _PlatformFilterChipState extends State<_PlatformFilterChip> | ||
| with SingleTickerProviderStateMixin { | ||
| bool _hovered = false; | ||
| bool _pressed = false; | ||
|
|
||
| // Short pop on tap — gsap.to(scale: 1.1, duration: 0.15) → scale: 1 | ||
| late final AnimationController _tapCtrl; | ||
| late final Animation<double> _tapScale; | ||
|
|
||
| @override | ||
| void initState() { | ||
| super.initState(); | ||
| _tapCtrl = AnimationController( | ||
| vsync: this, | ||
| duration: const Duration(milliseconds: 220), | ||
| value: 1.0, | ||
| ); | ||
| _tapScale = Tween<double>(begin: 1.0, end: 1.0).animate(_tapCtrl); | ||
| } | ||
|
|
||
| @override | ||
| void dispose() { | ||
| _tapCtrl.dispose(); | ||
| super.dispose(); | ||
| } | ||
|
|
||
| void _onTap() { | ||
| // Quick spring-back pop on press | ||
| _tapCtrl.duration = const Duration(milliseconds: 110); | ||
| _tapCtrl.reverse(from: 0.92); | ||
| Future.delayed(const Duration(milliseconds: 110), () { | ||
| if (mounted) { | ||
| _tapCtrl.duration = const Duration(milliseconds: 220); | ||
| _tapCtrl.forward(from: 1.0); | ||
| } | ||
| }); | ||
| widget.onTap(); | ||
| } |
There was a problem hiding this comment.
The tap animation for _PlatformFilterChip is non-functional. The _tapScale animation is initialized with a static Tween<double>(begin: 1.0, end: 1.0) which never changes, and the _pressed state variable is never updated to true or false.
We can fix this by removing the unused _pressed variable, defining _tapScale with a proper scale range (e.g., 0.94 to 1.0), and using _tapCtrl.animateTo in _onTap to smoothly transition the scale down and back up.
class _PlatformFilterChipState extends State<_PlatformFilterChip>
with SingleTickerProviderStateMixin {
bool _hovered = false;
late final AnimationController _tapCtrl;
late final Animation<double> _tapScale;
@override
void initState() {
super.initState();
_tapCtrl = AnimationController(
vsync: this,
value: 1.0,
);
_tapScale = Tween<double>(begin: 0.94, end: 1.0).animate(
CurvedAnimation(parent: _tapCtrl, curve: Curves.easeOut),
);
}
@override
void dispose() {
_tapCtrl.dispose();
super.dispose();
}
void _onTap() {
_tapCtrl.animateTo(0.0, duration: const Duration(milliseconds: 110));
Future.delayed(const Duration(milliseconds: 110), () {
if (mounted) {
_tapCtrl.animateTo(1.0, duration: const Duration(milliseconds: 220));
}
});
widget.onTap();
}| child: AnimatedScale( | ||
| scale: _pressed ? 0.94 : 1.0, | ||
| duration: const Duration(milliseconds: 120), | ||
| curve: Curves.easeOut, | ||
| child: Row( |
There was a problem hiding this comment.
Replace the non-functional AnimatedScale (which relies on the static _pressed variable) with a ScaleTransition bound to the corrected _tapScale animation.
| child: AnimatedScale( | |
| scale: _pressed ? 0.94 : 1.0, | |
| duration: const Duration(milliseconds: 120), | |
| curve: Curves.easeOut, | |
| child: Row( | |
| child: ScaleTransition( | |
| scale: _tapScale, | |
| child: Row( |
| _buildDivider(isDark), | ||
| // Per-platform mini-bars | ||
| ..._buildPlatformCountBars(isDark), |
There was a problem hiding this comment.
There is a duplicate vertical divider rendered between the 'Fatal crash' column and the first platform column. This happens because _buildPlatformCountBars already adds a divider as its first element, but an explicit _buildDivider(isDark) is also placed right before it in the Row children. Removing the explicit divider resolves this visual bug.
| _buildDivider(isDark), | |
| // Per-platform mini-bars | |
| ..._buildPlatformCountBars(isDark), | |
| // Per-platform mini-bars | |
| ..._buildPlatformCountBars(isDark), |
| return Transform.scale( | ||
| scale: 0.6 + 0.8 * _ctrl.value, | ||
| child: Opacity( | ||
| opacity: 0.6 * (1 - _ctrl.value), | ||
| child: Container( | ||
| width: widget.size, | ||
| height: widget.size, | ||
| decoration: BoxDecoration( | ||
| shape: BoxShape.circle, | ||
| color: widget.color.withValues(alpha: 0.6), | ||
| ), | ||
| ), | ||
| ), | ||
| ); |
There was a problem hiding this comment.
Using the Opacity widget inside an AnimatedBuilder for a perpetual animation is inefficient because it triggers intermediate offscreen render passes. Since the child is a simple Container with a solid color, we can animate the color's alpha directly in the BoxDecoration and remove the Opacity widget entirely.
| return Transform.scale( | |
| scale: 0.6 + 0.8 * _ctrl.value, | |
| child: Opacity( | |
| opacity: 0.6 * (1 - _ctrl.value), | |
| child: Container( | |
| width: widget.size, | |
| height: widget.size, | |
| decoration: BoxDecoration( | |
| shape: BoxShape.circle, | |
| color: widget.color.withValues(alpha: 0.6), | |
| ), | |
| ), | |
| ), | |
| ); | |
| return Transform.scale( | |
| scale: 0.6 + 0.8 * _ctrl.value, | |
| child: Container( | |
| width: widget.size, | |
| height: widget.size, | |
| decoration: BoxDecoration( | |
| shape: BoxShape.circle, | |
| color: widget.color.withValues(alpha: 0.6 * (1 - _ctrl.value)), | |
| ), | |
| ), | |
| ); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/features/error_inspector/presentation/pages/error_inspector_page.dart (1)
1094-1213: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winTap-pop animation is wired up but never actually animates anything.
_tapCtrl/_tapScaledrive the described "tap pop effect," but_tapScaleis built fromTween<double>(begin: 1.0, end: 1.0)(line 1111) — always evaluates to1.0regardless of controller value — and it's never referenced inbuild(). The actual scale shown,AnimatedScale(scale: _pressed ? 0.94 : 1.0, ...)(line 1177), depends on_pressed, which is declared but never set totrueanywhere (noonTapDown/onTapUp/onTapCancelhandlers on theGestureDetector). So the entire_onTap()pop sequence (lines 1120-1131) has no visible effect.🐛 Proposed fix: wire the tap-pop animation to the actual scale
_tapCtrl = AnimationController( vsync: this, duration: const Duration(milliseconds: 220), value: 1.0, ); - _tapScale = Tween<double>(begin: 1.0, end: 1.0).animate(_tapCtrl); + _tapScale = Tween<double>(begin: 0.92, end: 1.0).animate( + CurvedAnimation(parent: _tapCtrl, curve: Curves.easeOutCubic), + );- child: AnimatedScale( - scale: _pressed ? 0.94 : 1.0, - duration: const Duration(milliseconds: 120), - curve: Curves.easeOut, - child: Row( + child: AnimatedBuilder( + animation: _tapScale, + builder: (context, child) => Transform.scale( + scale: _tapScale.value, + child: child, + ), + child: Row(🤖 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 `@lib/features/error_inspector/presentation/pages/error_inspector_page.dart` around lines 1094 - 1213, The tap-pop animation in _PlatformFilterChipState is currently disconnected from what build() renders, so the press effect never shows. Wire the GestureDetector’s press lifecycle (onTapDown/onTapUp/onTapCancel) to _pressed, or replace the unused AnimatedScale state with the _tapCtrl/_tapScale animation. Also fix _tapScale so it actually changes scale instead of using a 1.0-to-1.0 tween, and ensure _onTap() drives the same visible scale path.
🧹 Nitpick comments (1)
lib/features/error_inspector/presentation/pages/error_inspector_page.dart (1)
736-750: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the unused platform-count helpers.
_buildPlatformCounts()and_CountChipno longer appear to be called now that the view uses_buildPlatformCountBars, so deleting them will keep the page from carrying dead UI logic.🤖 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 `@lib/features/error_inspector/presentation/pages/error_inspector_page.dart` around lines 736 - 750, Remove the dead platform-count UI helpers from error_inspector_page.dart: `_buildPlatformCounts()` and `_CountChip` are no longer used now that `_buildPlatformCountBars` drives the view. Delete those unused members and any related imports or references so `ErrorInspectorPage` only contains the active platform display logic.
🤖 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 `@lib/features/error_inspector/presentation/pages/error_inspector_page.dart`:
- Around line 1152-1153: The tooltip message in ErrorInspectorPage is hardcoded
and bypasses localization. Update the Tooltip in the widget build logic to use
S.of(context) for the “Hide/Show” wording and the “errors” label, matching the
other localized strings in this file. Keep the existing widget.isActive and
widget.label behavior, but build the full tooltip text from localized resources
instead of raw English literals.
---
Outside diff comments:
In `@lib/features/error_inspector/presentation/pages/error_inspector_page.dart`:
- Around line 1094-1213: The tap-pop animation in _PlatformFilterChipState is
currently disconnected from what build() renders, so the press effect never
shows. Wire the GestureDetector’s press lifecycle
(onTapDown/onTapUp/onTapCancel) to _pressed, or replace the unused AnimatedScale
state with the _tapCtrl/_tapScale animation. Also fix _tapScale so it actually
changes scale instead of using a 1.0-to-1.0 tween, and ensure _onTap() drives
the same visible scale path.
---
Nitpick comments:
In `@lib/features/error_inspector/presentation/pages/error_inspector_page.dart`:
- Around line 736-750: Remove the dead platform-count UI helpers from
error_inspector_page.dart: `_buildPlatformCounts()` and `_CountChip` are no
longer used now that `_buildPlatformCountBars` drives the view. Delete those
unused members and any related imports or references so `ErrorInspectorPage`
only contains the active platform display logic.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ac133a48-a448-4742-bfc1-49aa4c81fb69
📒 Files selected for processing (1)
lib/features/error_inspector/presentation/pages/error_inspector_page.dart
| return Tooltip( | ||
| message: '${widget.isActive ? "Hide" : "Show"} ${widget.label} errors', |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Hardcoded, non-localized tooltip text.
'${widget.isActive ? "Hide" : "Show"} ${widget.label} errors' bypasses S.of(context), unlike every other user-facing string in this file (e.g. S.of(context).errors, S.of(context).autoScroll). This breaks localization for this tooltip.
🤖 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 `@lib/features/error_inspector/presentation/pages/error_inspector_page.dart`
around lines 1152 - 1153, The tooltip message in ErrorInspectorPage is hardcoded
and bypasses localization. Update the Tooltip in the widget build logic to use
S.of(context) for the “Hide/Show” wording and the “errors” label, matching the
other localized strings in this file. Keep the existing widget.isActive and
widget.label behavior, but build the full tooltip text from localized resources
instead of raw English literals.
…lter tap interaction
1 participant
…ut and pulsing status indicators
Description
Related Issue
Type of Change
Testing
Screenshots (if applicable)
Summary by CodeRabbit
New Features
Bug Fixes