fix(analytics-dashboard): UI/UX adjustments for banner, metric cards, sparkline legend, and default tab

[oidacra]

Summary

Closes #34992

• Banner: Moved info banner outside header-container so it spans full width with no top/right/left margins
• Metric cards: Added "from previous X days" comparison label below trend badges (shows "from previous range" for custom date ranges)
• Trend badges: Added colored backgrounds — green ($color-palette-green-op-10) for positive, red ($color-palette-red-op-10) for negative
• Sparkline legend: Added footer legend showing "Current Period" (solid line) and "Previous Period" (dashed line)
• Dashboard max-width: Set max-width: 1600px with margin: 0 auto for centered content
• Default tab: Changed from Pageview to Engagement in with-filters.feature.ts
• Previous period line: Made sparkline previous period line dashed to match design
• Card styling: Added border and removed shadow in theme config

Changed Files

Data Access

• dot-analytics.constants.ts — Added TIME_RANGE_DAYS_MAP for comparison labels
• with-filters.feature.ts — Changed default tab to engagement
• filters.utils.ts — Added getComparisonLabel() utility
• filters.utils.spec.ts — Added tests for getComparisonLabel()

Dashboard

• dot-analytics-dashboard.component.html — Moved banner, added dashboard-content wrapper
• dot-analytics-dashboard.component.scss — Added max-width + centering
• dot-analytics-dashboard.component.spec.ts — Updated tests for new default tab

Metric Component

• dot-analytics-metric.component.ts — Added comparisonLabel input
• dot-analytics-metric.component.html — Added comparison label + restructured trend display
• dot-analytics-metric.component.scss — Added trend backgrounds, comparison label styles

Sparkline Component

• dot-analytics-sparkline.component.ts — Added footer legend, $legendItems computed

Report Components (Engagement, Pageview, Conversions)

• Added $comparisonLabel computed signal and passed to metric cards
• Made previous period sparkline line dashed

Theme

• theme.config.ts — Card border + no shadow

Test plan

• yarn nx test portlets-dot-analytics — 245 tests pass
• yarn nx test portlets-dot-analytics-data-access — 161 tests pass
• yarn nx lint portlets-dot-analytics — passes
• yarn nx lint portlets-dot-analytics-data-access — passes
• Visual verification: banner full-width, trend backgrounds, sparkline legend, max-width container
• Verify default tab is Engagement on fresh load (no ?tab= param)
• Verify "from previous 7 days" / "from previous 30 days" / "from previous range" text

This PR fixes: #34992

[claude]

Claude finished @oidacra's task in 2m 9s —— View job

---

Claude finished reviewing this PR

---

Code Review

Three things worth fixing:

---

CommonModule unused import — dot-analytics-conversions-report.component.ts:44

The template uses @for/@if modern control flow, so CommonModule is no longer needed. Dead import.

Fix this →

---

Duplicate icon on total-sessions metric — dot-analytics-engagement-report.component.html:50

Both "Avg Interactions" (line 35) and "Total Sessions" (line 50) use icon="pi-bullseye". Total Sessions should use a distinct icon (e.g., pi-users).

Fix this →

---

Sparkline legend bleed has fragile coupling — dot-analytics-sparkline.component.ts:217

margin: 0.5rem -1rem -1rem;

This negative margin assumes the parent card body padding is exactly 1rem. That value is set in theme.config.ts (body.padding: '1rem'), but the two are not linked — if the card padding changes, the legend bleed breaks silently. At minimum, add a comment documenting the assumption:

// Bleeds into p-card body padding (theme.config.ts card.body.padding = 1rem)
margin: 0.5rem -1rem -1rem;

---

Everything else is clean: getComparisonLabel edge case is properly handled with days !== undefined, i18n integration is correct, global card styling is intentional, and component patterns follow project conventions.

[claude]

Claude finished @oidacra's task —— View job

---

Claude is reviewing this PR

Todo List

• Review data access layer changes (constants, utils, store)
• Review dashboard component modifications
• Review metric component changes
• Review sparkline component updates
• Review report component modifications
• Review theme configuration changes
• Analyze test coverage and quality
• Check for security, performance, or design issues
• Post comprehensive review feedback

---

Code Review Results

After systematically reviewing all 19 changed files, I've identified several issues that warrant attention:

🚨 Critical Issue: Edge Case in Utility Function

Location: core-web/libs/portlets/dot-analytics/data-access/src/lib/utils/filters.utils.ts:64

const days = TIME_RANGE_DAYS_MAP[timeRange];
return days ? `from previous ${days} days` : 'from previous range';

Problem: When timeRange is neither an array nor found in TIME_RANGE_DAYS_MAP, the function returns "from previous undefined days" due to template literal execution.

Fix needed: The current fix attempts to handle this, but the original issue remains. The logic should be:

export function getComparisonLabel(timeRange: TimeRangeInput): string {
    if (Array.isArray(timeRange)) {
        return 'from previous range';
    }
    
    const days = TIME_RANGE_DAYS_MAP[timeRange];
    return days !== undefined ? `from previous ${days} days` : 'from previous range';
}

⚠️ Global Theme Impact Risk

Location: core-web/libs/ui/src/lib/theme/theme.config.ts:37-47

Issue: The card styling changes affect ALL cards globally, not just analytics components:

• Added border: border: 1px solid dt('gray.300')
• Removed shadow: shadow: 'none'

Risk: This could break the visual design of cards throughout the entire application. Other components may depend on the default shadow behavior or expect borderless cards.

Recommendation: Consider scoping these styles to analytics-specific cards or verify impact across all card usage.

🔍 Layout Structure Change

Location: dot-analytics-dashboard.component.html:2-16

Issue: The banner was moved outside the header-container but lacks a proper wrapper container. This could cause layout issues if the parent component doesn't handle padding/margins properly.

Recommendation: Consider wrapping the banner in a container or ensuring parent component layout compatibility.

📝 Missing Test Coverage

Location: dot-analytics-metric.component.ts:50

Issue: The new comparisonLabel input is not directly tested in isolation. While it's tested through integration in report components, there are no direct unit tests for:

• The input binding behavior
• Edge cases with empty/null comparison labels
• Proper display in different states (loading, error, empty)

---

Positive Observations

• Well-structured component changes: The sparkline legend implementation follows good Angular patterns
• Consistent styling: Uses SCSS variables appropriately for trend backgrounds
• Good separation of concerns: Utility function is properly tested with comprehensive edge cases
• Type safety: All new properties are properly typed with Angular signals

---

Summary

High Priority: Fix the getComparisonLabel() utility function edge case and evaluate global card theme impact.

Medium Priority: Add direct unit tests for the new comparisonLabel input and review banner container structure.

The implementation is generally solid but needs attention to the critical utility function bug and potential global styling impact.

---

[oidacra]

Claude finished @oidacra's task —— View job

Claude is reviewing this PR

Todo List

• Review data access layer changes (constants, utils, store)
• Review dashboard component modifications
• Review metric component changes
• Review sparkline component updates
• Review report component modifications
• Review theme configuration changes
• Analyze test coverage and quality
• Check for security, performance, or design issues
• Post comprehensive review feedback

Code Review Results

After systematically reviewing all 19 changed files, I've identified several issues that warrant attention:

🚨 Critical Issue: Edge Case in Utility Function

Location: core-web/libs/portlets/dot-analytics/data-access/src/lib/utils/filters.utils.ts:64

const days = TIME_RANGE_DAYS_MAP[timeRange];
return days ? `from previous ${days} days` : 'from previous range';

Problem: When timeRange is neither an array nor found in TIME_RANGE_DAYS_MAP, the function returns "from previous undefined days" due to template literal execution.

Fix needed: The current fix attempts to handle this, but the original issue remains. The logic should be:

export function getComparisonLabel(timeRange: TimeRangeInput): string {
    if (Array.isArray(timeRange)) {
        return 'from previous range';
    }
    
    const days = TIME_RANGE_DAYS_MAP[timeRange];
    return days !== undefined ? `from previous ${days} days` : 'from previous range';
}

⚠️ Global Theme Impact Risk

Location: core-web/libs/ui/src/lib/theme/theme.config.ts:37-47

Issue: The card styling changes affect ALL cards globally, not just analytics components:

• Added border: border: 1px solid dt('gray.300')
• Removed shadow: shadow: 'none'

Risk: This could break the visual design of cards throughout the entire application. Other components may depend on the default shadow behavior or expect borderless cards.

Recommendation: Consider scoping these styles to analytics-specific cards or verify impact across all card usage.

🔍 Layout Structure Change

Location: dot-analytics-dashboard.component.html:2-16

Issue: The banner was moved outside the header-container but lacks a proper wrapper container. This could cause layout issues if the parent component doesn't handle padding/margins properly.

Recommendation: Consider wrapping the banner in a container or ensuring parent component layout compatibility.

📝 Missing Test Coverage

Location: dot-analytics-metric.component.ts:50

Issue: The new comparisonLabel input is not directly tested in isolation. While it's tested through integration in report components, there are no direct unit tests for:

• The input binding behavior
• Edge cases with empty/null comparison labels
• Proper display in different states (loading, error, empty)

Positive Observations

• Well-structured component changes: The sparkline legend implementation follows good Angular patterns
• Consistent styling: Uses SCSS variables appropriately for trend backgrounds
• Good separation of concerns: Utility function is properly tested with comprehensive edge cases
• Type safety: All new properties are properly typed with Angular signals

Summary

High Priority: Fix the getComparisonLabel() utility function edge case and evaluate global card theme impact.

Medium Priority: Add direct unit tests for the new comparisonLabel input and review banner container structure.

The implementation is generally solid but needs attention to the critical utility function bug and potential global styling impact.

▎ 1. Global card styling in theme.config.ts — This is intentional. The card style (no shadow + border) is the desired global design direction, not analytics-specific. No change needed.
▎ 2. Comparison labels not i18n — The getComparisonLabel() values come from a select with only two options (7 and 30 days) plus a custom range fallback. The days !== undefined guard already handles the edge
case correctly. These labels are fine as-is for now.
▎ 3. pb-0 on sparkline legend — Removed. The Tailwind utility had no effect due to component style specificity. (fixed in latest commit)
▎ 4. Negative margins on sparkline legend — Intentional. The legend needs to bleed into the card padding for the desired visual effect.
▎ 5. Missing engagement tab in isValidTab test — Added. (fixed in latest commit)
▎ 6. Missing comparisonLabel input tests — Added three tests covering: label with trend data, no label provided, and no-prior-data state. (fixed in latest commit)
▎ 7. Previous period sparkline truncation — Informational only, no action needed.

changes done.