Conversation
peterzimon
commented
Mar 19, 2025
- Fixed "NaN" output for bounce rate and visit duration
- Fixed jumping dropdowns
- Fixed "NaN" output for bounce rate and visit duration - Fixed jumping dropdowns
WalkthroughThe pull request updates the formatting logic within the Possibly related PRs
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
ghost/admin/app/components/stats/kpis-overview.jsOops! Something went wrong! :( ESLint: 8.44.0 Error: Failed to load parser '@babel/eslint-parser' declared in 'ghost/admin/.eslintrc.js': Cannot find module '@babel/eslint-parser'
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
ghost/admin/app/components/stats/kpis-overview.js(1 hunks)ghost/admin/app/templates/stats.hbs(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
ghost/admin/app/components/stats/kpis-overview.js (1)
ghost/admin/app/utils/stats.js (1) (1)
formatVisitDuration(133-154)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Admin tests - Chrome
🔇 Additional comments (6)
ghost/admin/app/templates/stats.hbs (3)
5-12: Button formatting looks good.The code formatting adjustments improve readability while preserving functionality.
14-19: Appropriate container for AudienceFilter.Wrapping the AudienceFilter component in a div container helps prevent UI jumping during rendering and interaction, which aligns with the PR objective of fixing jumping dropdowns.
21-36: Good dropdown container implementation.Adding a container div around the PowerSelect component helps stabilize its position in the UI, preventing it from jumping during interactions. This addresses one of the main objectives of the PR.
ghost/admin/app/components/stats/kpis-overview.js (3)
99-101: Good variable extraction for formatted values.Extracting these calculations into named variables improves code readability and makes the return statement cleaner.
104-105: Good default handling for numeric metrics.Using the
|| '0'pattern ensures that falsy values (includingundefined,null, or empty string) are replaced with a default of '0', preventing empty or NaN displays.
106-106: Proper bounce rate formatting.The conditional logic correctly handles NaN and falsy values for bounce rate, which addresses one of the main objectives of the PR to fix NaN outputs.
| pageviews: formatNumber(_KPITotal('pageviews')), | ||
| visits: formatNumber(totalVisits), | ||
| bounce_rate: (_ponderatedKPIsTotal('bounce_rate') * 100).toFixed(0) | ||
| avg_session_sec: isNaN(formattedVisitDurations) || formattedVisitDurations ? '0m' : formattedVisitDurations, |
There was a problem hiding this comment.
Fix conditional logic for avg_session_sec.
There's a logical error in this condition. The current implementation will always return '0m' when formattedVisitDurations is truthy, which is the opposite of what's intended.
-avg_session_sec: isNaN(formattedVisitDurations) || formattedVisitDurations ? '0m' : formattedVisitDurations,
+avg_session_sec: isNaN(formattedVisitDurations) || !formattedVisitDurations ? '0m' : formattedVisitDurations,📝 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.
| avg_session_sec: isNaN(formattedVisitDurations) || formattedVisitDurations ? '0m' : formattedVisitDurations, | |
| avg_session_sec: isNaN(formattedVisitDurations) || !formattedVisitDurations ? '0m' : formattedVisitDurations, |