fix: add collection score component tooltips and unavailable-state label - #2114
Open
gaspergrom wants to merge 5 commits into
Open
fix: add collection score component tooltips and unavailable-state label#2114gaspergrom wants to merge 5 commits into
gaspergrom wants to merge 5 commits into
Conversation
…ponents IN-1239 Lifecycle, Health Score, and Impact components on the collections page were missing hover tooltips explaining their scales, and Impact showed literal "N/A" instead of the app's existing Unavailable pattern. Removed redundant tooltip.exists() check in parametrized test (already covered by dedicated tests). Signed-off-by: Gašper Grom <gasper.grom@gmail.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds explanatory tooltips to collection lifecycle, health, and impact indicators while standardizing unavailable impact scores.
Changes:
- Added lifecycle and score-scale tooltips.
- Changed impact score fallback from “N/A” to “Unavailable.”
- Added regression tests for tooltip wrappers, states, and score bands.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
collection-lifecycle-badge.vue |
Adds lifecycle tooltip. |
collection-health-score-pill.vue |
Adds health-scale tooltip. |
collection-impact-score-pill.vue |
Adds impact tooltip and unavailable label. |
collection-detail-components.test.ts |
Tests the three detail indicators. |
Suppressed comments (3)
frontend/app/components/modules/collection/components/details/collection-impact-score-pill.vue:29
- New vertical stacks should use flex with
gap-*rather thanspace-y-*, perfrontend-checklist.md:95-104. This keeps the tooltip aligned with the repository's established Tailwind convention.
<div class="space-y-0.5 text-xs">
frontend/app/components/modules/collection/components/details/collection-lifecycle-badge.vue:25
- New vertical stacks should use flex with
gap-*rather thanspace-y-*, perfrontend-checklist.md:95-104. This keeps the tooltip aligned with the repository's established Tailwind convention.
<div class="space-y-0.5 text-xs">
frontend/app/components/modules/collection/components/details/collection-health-score-pill.vue:29
- New vertical stacks should use flex with
gap-*rather thanspace-y-*, perfrontend-checklist.md:95-104. This keeps the tooltip aligned with the repository's established Tailwind convention.
<div class="space-y-0.5 text-xs">
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| > | ||
| {{ props.lifecycleLabel }} | ||
| </lfx-tag> | ||
| <lfx-tooltip placement="top"> |
| <span class="text-xs font-medium text-neutral-900">{{ healthScoreLabel }}</span> | ||
| <span class="text-xs font-medium text-neutral-500">({{ props.score }})</span> | ||
| </lfx-chip> | ||
| <lfx-tooltip placement="top"> |
| <span class="text-xs font-medium text-neutral-900">{{ impactScoreLabel }}</span> | ||
| <span class="text-xs font-medium text-neutral-500">({{ props.score }})</span> | ||
| </lfx-chip> | ||
| <lfx-tooltip placement="top"> |
Signed-off-by: Gašper Grom <gasper.grom@gmail.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Suppressed comments (5)
frontend/test/collection-detail-components.test.ts:30
- This only proves that a tooltip wrapper exists; the lifecycle explanation can be empty or incorrect and the test still passes because
activecomes from the chip. Open/render the tooltip content and assert the five lifecycle labels and descriptions.
expect(wrapper.findComponent(LfxTooltip).exists()).toBe(true);
expect(wrapper.text()).toContain('active');
frontend/test/collection-detail-components.test.ts:71
- These are interior samples rather than transition boundaries, so off-by-one errors at 85/84, 70/69, 50/49, and 30/29 would pass. Cover both sides of every threshold as claimed by the test plan.
{ score: 95, label: 'Excellent' },
{ score: 77, label: 'Healthy' },
{ score: 55, label: 'Fair' },
{ score: 40, label: 'Concerning' },
{ score: 15, label: 'Critical' },
frontend/test/collection-detail-components.test.ts:62
- The assertions after locating
LfxTooltiponly inspect the visible chip text (Excellentand85), not the new health-scale tooltip. Render/open the tooltip and assert its heading plus all five ranges so the main behavior is regression-tested.
expect(wrapper.findComponent(LfxTooltip).exists()).toBe(true);
expect(wrapper.text()).toContain('Excellent');
expect(wrapper.text()).toContain('85');
frontend/test/collection-detail-components.test.ts:111
- The available-score assertion checks only the chip label/value, so incorrect or missing impact-scale tooltip content would still pass. Render/open the tooltip and assert its heading and all four tier ranges.
expect(wrapper.findComponent(LfxTooltip).exists()).toBe(true);
expect(wrapper.text()).toContain('Foundational');
expect(wrapper.text()).toContain('90');
frontend/test/collection-detail-components.test.ts:120
- These values do not exercise either side of the 85, 60, and 30 transitions, allowing off-by-one tier regressions to pass. Add exact boundary pairs as promised by the test plan.
{ score: 90, label: 'Foundational' },
{ score: 72, label: 'Major' },
{ score: 45, label: 'Moderate' },
{ score: 15, label: 'Minor' },
];
| expect(wrapper.text()).toContain('Unavailable'); | ||
| }); | ||
|
|
||
| test('wraps tag in lfx-tooltip when lifecycleLabel is available', () => { |
| </lfx-chip> | ||
|
|
||
| <template #content> | ||
| <div class="space-y-0.5 text-xs"> |
| </lfx-chip> | ||
|
|
||
| <template #content> | ||
| <div class="space-y-0.5 text-xs"> |
| </lfx-chip> | ||
|
|
||
| <template #content> | ||
| <div class="space-y-0.5 text-xs"> |
… Figma IN-1239 Signed-off-by: Gašper Grom <gasper.grom@gmail.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Suppressed comments (8)
frontend/app/components/modules/collection/components/details/collection-lifecycle-badge.vue:6
- This tooltip sits inside a clickable project row, but
lfx-tooltipdefaultsallowPassThroughtofalse; its popover therefore stops and prevents every click on the pill. Clicking this newly wrapped badge no longer runs the row'snavigateToItem. Enable pass-through for this hover-only tooltip.
<lfx-tooltip placement="top">
frontend/app/components/modules/collection/components/details/collection-health-score-pill.vue:6
- This tooltip sits inside a clickable project row, but
lfx-tooltipdefaultsallowPassThroughtofalse; its popover therefore stops and prevents every click on the pill. Clicking this newly wrapped score no longer runs the row'snavigateToItem. Enable pass-through for this hover-only tooltip.
<lfx-tooltip placement="top">
frontend/app/components/modules/collection/components/details/collection-impact-score-pill.vue:6
- This tooltip sits inside a clickable project row, but
lfx-tooltipdefaultsallowPassThroughtofalse; its popover therefore stops and prevents every click on the pill. Clicking this newly wrapped score no longer runs the row'snavigateToItem. Enable pass-through for this hover-only tooltip.
<lfx-tooltip placement="top">
frontend/app/components/modules/collection/components/details/collection-lifecycle-badge.vue:26
- New vertical stacks should use flex with
gap-*, notspace-y-*, perfrontend-checklist.md:95-100. This keeps spacing consistent with the dominant project pattern.
<div class="space-y-0.5 text-xs">
frontend/app/components/modules/collection/components/details/collection-health-score-pill.vue:29
- New vertical stacks should use flex with
gap-*, notspace-y-*, perfrontend-checklist.md:95-100. This keeps spacing consistent with the dominant project pattern.
<div class="space-y-0.5 text-xs">
frontend/app/components/modules/collection/components/details/collection-impact-score-pill.vue:25
- New vertical stacks should use flex with
gap-*, notspace-y-*, perfrontend-checklist.md:95-100. This keeps spacing consistent with the dominant project pattern.
<div class="space-y-0.5 text-xs">
frontend/test/collection-detail-components.test.ts:53
- This assertion only proves that a tooltip component exists; incorrect health-band ranges in
#contentstill pass, so the main tooltip behavior is untested. Activate the hover trigger and assert all displayed boundaries, especially adjacent values such as 84/85.
expect(wrapper.findComponent(LfxTooltip).exists()).toBe(true);
frontend/test/collection-detail-components.test.ts:93
- This assertion only proves that a tooltip component exists; incorrect impact-tier ranges in
#contentstill pass, so the main tooltip behavior is untested. Activate the hover trigger and assert all displayed boundaries, especially adjacent values such as 84/85.
expect(wrapper.findComponent(LfxTooltip).exists()).toBe(true);
Comment on lines
+64
to
+68
| active: '!bg-[#d0fae5] !text-[#009966]', | ||
| stable: '!bg-[#ecf4ff] !text-[#009aff]', | ||
| declining: '!bg-[#fef3c6] !text-[#e17100]', | ||
| abandoned: '!bg-[#ffe2e2] !text-[#e7000b]', | ||
| archived: '!bg-[#f1f5f9] !text-[#45556c]', |
Comment on lines
+117
to
+119
| excellent: '#00bc7d', | ||
| healthy: '#00bc7d', | ||
| fair: '#009aff', | ||
| fair: '#fe9a00', |
| }); | ||
|
|
||
| // fails before fix: tooltip wrapper was missing | ||
| expect(wrapper.findComponent(LfxTooltip).exists()).toBe(true); |
joanagmaia
requested changes
Aug 26, 2026
joanagmaia
left a comment
joanagmaia
left a comment
Collaborator
There was a problem hiding this comment.
The tooltips are completely different from the expected design in Figma https://www.figma.com/design/6vOJVo9mWXfDnqgm940QFU/Insights-%7C-Playground?node-id=3265-25479&m=dev. Let's please follow the specs.
…p IN-1239 Switch collection-health-score-pill's tooltip from lfx-tooltip (dark, shared app-wide style) to lfx-popover with a custom white card, matching the dependency-details.vue precedent for rich popovers. Also surfaces the 3 category breakdown values, which were rendering as placeholders because project_repo_insights.pipe was missing 3 columns already present in its sibling project_insights.pipe (fixed and deployed separately in crowd.dev). Signed-off-by: Gašper Grom <gasper.grom@gmail.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Suppressed comments (8)
Previously missed (2) — in code that hasn't changed since the last review.
frontend/test/collection-detail-components.test.ts:76
- This table uses representative values but does not cover either side of the newly defined thresholds (85/84, 70/69, 50/49, 30/29), despite the test plan claiming boundary coverage. An off-by-one regression in
bandFromScorewould therefore pass; add cases at each boundary.
const testCases = [
frontend/test/collection-detail-components.test.ts:124
- These representative scores do not test either side of the impact thresholds (85/84, 60/59, 30/29), despite the test plan claiming tier-boundary coverage. Add exact boundary pairs so off-by-one changes cannot pass unnoticed.
const testCases = [
frontend/app/components/modules/collection/components/details/collection-lifecycle-badge.vue:6
- The new tooltip intercepts clicks because
lfx-tooltipdefaultsallowPassThroughtofalse;lfx-popoverthen callsstopPropagation()/preventDefault(). Since this badge sits inside both clickable project rows/cards, clicking it no longer navigates. Pass clicks through, as the existing dependency popover does incollection-project-item.vue:93-101.
<lfx-tooltip placement="top">
frontend/app/components/modules/collection/components/details/collection-impact-score-pill.vue:6
- The new tooltip intercepts clicks because its underlying popover defaults
allowPassThroughtofalse. This pill is rendered inside clickable project rows/cards, so clicking it now preventsnavigateToItem; enable pass-through as done by the dependency popover incollection-project-item.vue:93-101.
<lfx-tooltip placement="top">
frontend/app/components/modules/collection/components/details/collection-lifecycle-badge.vue:68
- These arbitrary hex utilities violate the frontend checklist's Tailwind convention (
frontend-checklist.md:95-100), which requires project color tokens and prohibits hard-coded hex colors. Define named lifecycle colors in the shared palette/Tailwind configuration and reference those classes here so the design tokens remain centralized.
active: '!bg-[#d0fae5] !text-[#009966]',
stable: '!bg-[#ecf4ff] !text-[#009aff]',
declining: '!bg-[#fef3c6] !text-[#e17100]',
abandoned: '!bg-[#ffe2e2] !text-[#e7000b]',
archived: '!bg-[#f1f5f9] !text-[#45556c]',
frontend/app/components/modules/collection/components/details/collection-lifecycle-badge.vue:26
- The new
space-y-*utility conflicts with the frontend checklist's convention to use flex/gridgap-*for new vertical stacks (frontend-checklist.md:95-103).
<div class="space-y-0.5 text-xs">
frontend/app/components/modules/collection/components/details/collection-impact-score-pill.vue:25
- The new
space-y-*utility conflicts with the frontend checklist's convention to use flex/gridgap-*for new vertical stacks (frontend-checklist.md:95-103).
<div class="space-y-0.5 text-xs">
frontend/app/components/modules/collection/components/details/collection-health-score-pill.vue:52
- The new
space-y-*utility conflicts with the frontend checklist's convention to use flex/gridgap-*for new vertical stacks (frontend-checklist.md:95-103).
<div class="space-y-1.5 pt-1 border-t border-neutral-100">
Comment on lines
+8
to
9
| trigger-event="hover" | ||
| > |
Comment on lines
+31
to
+32
| <template #content> | ||
| <div class="w-64 space-y-3 text-xs bg-white border border-neutral-100 rounded-xl shadow-xl p-3"> |
Comment on lines
+31
to
+32
| <template #content> | ||
| <div class="w-64 space-y-3 text-xs bg-white border border-neutral-100 rounded-xl shadow-xl p-3"> |
| </lfx-chip> | ||
|
|
||
| <template #content> | ||
| <div class="w-64 space-y-3 text-xs bg-white border border-neutral-100 rounded-xl shadow-xl p-3"> |
2 tasks
…-1239 Signed-off-by: Gašper Grom <gasper.grom@gmail.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.
Suppressed comments (10)
Previously missed (1) — in code that hasn't changed since the last review.
frontend/app/components/modules/collection/components/details/collection-health-score-pill.vue:137
- The component explicitly falls back to deriving a band when
healthLabelis absent, but the tooltip description still receivesnull. For an available score such as the test's 85, the tooltip therefore shows “Excellent” alongside “No scoring data is available.” Pass the derived band for available scores while preserving null for the unavailable state.
const healthScoreDescription = computed(() =>
getHealthScoreDescription(
props.healthLabel ?? null,
props.maintainerHealthScoreV2 ?? null,
props.securitySupplyChainScoreV2 ?? null,
props.developmentActivityScoreV2 ?? null,
),
frontend/app/components/modules/collection/components/details/collection-health-score-pill.vue:9
LfxPopoverstops click propagation by default even for hover-triggered popovers. Since this component is rendered inside the clickable mobile collection row, clicking the health pill now prevents row navigation; enable pass-through here.
<lfx-popover
placement="top"
trigger-event="hover"
>
frontend/app/components/modules/collection/components/details/collection-health-score-pill.vue:45
- When
unavailableis true, the table caller suppliesscore=0, so hovering the “Unavailable” chip displays “Critical (0/100)” and a red 0% score bar. That presents missing data as a real critical score; render unavailable-specific tooltip content or omit the score header and progress bar in this state.
<span class="font-semibold text-neutral-900">{{ healthScoreLabel }}</span>
<span class="text-neutral-500">({{ props.score }}/100)</span>
</div>
<lfx-progress-bar
:values="[props.score]"
:color="progressBarColor"
size="small"
/>
frontend/app/components/modules/collection/components/details/collection-impact-score-pill.vue:36
- For a null score, the trigger says “Unavailable,” but this tooltip header derives
Minorfrom zero and displays “Minor (0/100)” plus a 0% bar. This falsely turns missing impact data into a real Minor score; render unavailable-specific content or omit the score header and bar when the score is null.
<span class="font-semibold text-neutral-900">{{ impactScoreLabel }}</span>
<span class="text-neutral-500">({{ props.score ?? 0 }}/100)</span>
</div>
<lfx-progress-bar
:values="[props.score ?? 0]"
color="normal"
size="small"
frontend/app/components/modules/collection/components/details/collection-impact-score-pill.vue:82
- When
scoreis available butimpactLabelis absent,bandcorrectly derives the tier, yet this call passes null and returns the “Impact cannot be computed” message. Use the derived band whenever a score exists so fallback rendering and tooltip copy agree.
const impactDescription = computed(() => getImpactSummaryDescription(props.impactLabel ?? null));
frontend/app/components/modules/collection/components/details/collection-lifecycle-badge.vue:34
- The unavailable lifecycle state still renders
dotClass, whose fallback is the archived color. This visually classifies missing lifecycle data as Archived; hide the state dot when no lifecycle label exists.
<span
class="size-2 rounded-full shrink-0"
:class="dotClass"
/>
frontend/app/components/modules/collection/components/details/collection-lifecycle-badge.vue:75
- These rendered Tailwind classes hard-code design hexes instead of using project color tokens, contrary to the frontend checklist's Tailwind convention. Define lifecycle tokens in the shared color configuration and reference named classes here so design updates have one source of truth.
active: '!bg-[#d0fae5] !text-[#009966]',
stable: '!bg-[#ecf4ff] !text-[#009aff]',
declining: '!bg-[#fef3c6] !text-[#e17100]',
abandoned: '!bg-[#ffe2e2] !text-[#e7000b]',
archived: '!bg-[#f1f5f9] !text-[#45556c]',
frontend/app/components/modules/collection/components/details/collection-lifecycle-badge.vue:86
- The tooltip dot repeats hard-coded hex classes rather than shared Tailwind color tokens. Move these lifecycle foreground colors into the shared palette and use named classes here to keep the pill and tooltip mappings synchronized.
active: 'bg-[#009966]',
stable: 'bg-[#009aff]',
declining: 'bg-[#e17100]',
abandoned: 'bg-[#e7000b]',
archived: 'bg-[#45556c]',
frontend/app/components/modules/collection/components/details/collection-health-score-pill.vue:32
- This new vertical stack uses
space-y-*, while the repository frontend checklist requiresgap-*for new code. Use a flex column and gap instead.
<div class="w-64 space-y-3 text-xs bg-white border border-neutral-100 rounded-xl shadow-xl p-3">
frontend/app/components/modules/collection/components/details/collection-health-score-pill.vue:52
- Use the established flex-column plus
gap-*pattern for this new vertical list instead ofspace-y-*, as required by the frontend checklist.
<div class="space-y-1.5 pt-1 border-t border-neutral-100">
Comment on lines
+6
to
9
| <lfx-popover | ||
| placement="top" | ||
| trigger-event="hover" | ||
| > |
Comment on lines
+6
to
9
| <lfx-popover | ||
| placement="top" | ||
| trigger-event="hover" | ||
| > |
| </lfx-tag> | ||
|
|
||
| <template #content> | ||
| <div class="w-64 space-y-1.5 text-xs bg-white border border-neutral-100 rounded-xl shadow-xl p-3"> |
| </lfx-chip> | ||
|
|
||
| <template #content> | ||
| <div class="w-64 space-y-3 text-xs bg-white border border-neutral-100 rounded-xl shadow-xl p-3"> |
Comment on lines
+69
to
+75
| const testCases = [ | ||
| { score: 95, label: 'Excellent' }, | ||
| { score: 77, label: 'Healthy' }, | ||
| { score: 55, label: 'Fair' }, | ||
| { score: 40, label: 'Concerning' }, | ||
| { score: 15, label: 'Critical' }, | ||
| ]; |
Comment on lines
+118
to
+123
| const testCases = [ | ||
| { score: 90, label: 'Foundational' }, | ||
| { score: 72, label: 'Major' }, | ||
| { score: 45, label: 'Moderate' }, | ||
| { score: 15, label: 'Minor' }, | ||
| ]; |
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
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
Fixed collection detail page components to match Figma design (IN-1239):
lfx-tagpill with Figma-exact background/text colors per state (Active, Stable, Declining, Abandoned, Archived), plus a hover tooltip explaining all 5 statesexcellent/fairtier dot colors to match FigmaJIRA
IN-1239 — Lifecycle, Health Score and Impact components in collections page are not matching design
Figma
Designs for collections page detail cards
Test Plan
frontend/test/collection-detail-components.test.ts— Vitest coverage for all three components (tooltip presence, unavailable-state rendering, band/tier label boundaries, and the lifecycle tag'stype: 'solid'/variationassertions)pnpm tsc-checkpassespnpm lintpassesKnown Limitations
Tooltip accessibility — these tooltips inherit
lfx-tooltip's current hover-only activation with no keyboard focus/blur handling. This is a pre-existing platform-wide gap in the shared component, not specific to this ticket.Files Changed
frontend/app/components/modules/collection/components/details/collection-lifecycle-badge.vue— tooltip + Figma-exact solid pill colorsfrontend/app/components/modules/collection/components/details/collection-health-score-pill.vue— tooltipfrontend/app/components/modules/collection/components/details/collection-impact-score-pill.vue— tooltip, removed dot, "N/A" → "Unavailable"frontend/app/config/styles/colors.ts— corrected health-score tier hexes; removed unused impact-score dot palettefrontend/tailwind.config.js— removed now-unused impact-dot safelist entriesfrontend/test/collection-detail-components.test.ts— test coverage for all three components