From 4309b7f6067b1997909e41fb4f88ef3c96b3ba96 Mon Sep 17 00:00:00 2001 From: Chris0Jeky Date: Sat, 5 Sep 2026 08:01:48 +0100 Subject: [PATCH 1/4] fix(review): state an empty similar-past card before its disclosure opens MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The card put its entire empty state inside a collapsed region, so a reviewer looking at a proposal with no comparable history saw only "Show similar decisions" and had to open the control to learn there was nothing behind it. Hoist the empty sentence to the card, above the disclosure, and give the closed control an honest label ("Show similar decisions (none found)"). The sentence is not duplicated inside the region; the region gets a short line saying where comparable decisions will appear, so opening the control explains itself instead of revealing a zero-height void that the required E2E smoke asserts is visible. The disclosure and its toggle stay present in every state: PaperReviewView.spec.ts, PaperReviewView.language.spec.ts and tests/e2e/review-proposals.spec.ts all drive that button on an empty fixture, so removing it on empty is not available here. This layer cannot tell "still loading" from "loaded and empty" — it receives rows and an apply rate, nothing about the fetch. The empty copy is the honest reading of an empty array at this layer; distinguishing the two needs a prop from the rail and stays tracked on #1940. New keys land in en, es and it together (catalog parity guard). Refs #1940 --- .../taskdeck-web/src/locales/en/review.ts | 6 + .../taskdeck-web/src/locales/es/review.ts | 2 + .../taskdeck-web/src/locales/it/review.ts | 2 + .../paper/review/ReviewSimilarPast.spec.ts | 151 ++++++++++++++++++ .../views/paper/review/ReviewSimilarPast.vue | 42 ++++- 5 files changed, 197 insertions(+), 6 deletions(-) create mode 100644 frontend/taskdeck-web/src/tests/views/paper/review/ReviewSimilarPast.spec.ts diff --git a/frontend/taskdeck-web/src/locales/en/review.ts b/frontend/taskdeck-web/src/locales/en/review.ts index 203c1d65e..e53395351 100644 --- a/frontend/taskdeck-web/src/locales/en/review.ts +++ b/frontend/taskdeck-web/src/locales/en/review.ts @@ -475,8 +475,14 @@ export default { similarPast: { heading: 'Similar past decisions', empty: 'No comparable past decisions.', + // Shown inside the disclosure when there is nothing to list, so opening the + // control explains itself instead of revealing an empty region (#1940). + emptyDetail: 'Decisions on comparable proposals will be listed here.', details: { show: 'Show similar decisions', + // The empty-state label. It keeps the `show` wording and adds the count, + // so the closed control does not promise a list it does not have. + showEmpty: 'Show similar decisions (none found)', hide: 'Hide similar decisions', }, verdict: { diff --git a/frontend/taskdeck-web/src/locales/es/review.ts b/frontend/taskdeck-web/src/locales/es/review.ts index df00fdecc..3115e09fa 100644 --- a/frontend/taskdeck-web/src/locales/es/review.ts +++ b/frontend/taskdeck-web/src/locales/es/review.ts @@ -402,8 +402,10 @@ export default { similarPast: { heading: 'Decisiones parecidas anteriores', empty: 'No hay decisiones anteriores comparables.', + emptyDetail: 'Aquí se mostrarán las decisiones sobre propuestas comparables.', details: { show: 'Mostrar decisiones parecidas', + showEmpty: 'Mostrar decisiones parecidas (ninguna encontrada)', hide: 'Ocultar decisiones parecidas', }, verdict: { diff --git a/frontend/taskdeck-web/src/locales/it/review.ts b/frontend/taskdeck-web/src/locales/it/review.ts index c94f1876b..d61432e10 100644 --- a/frontend/taskdeck-web/src/locales/it/review.ts +++ b/frontend/taskdeck-web/src/locales/it/review.ts @@ -403,8 +403,10 @@ export default { similarPast: { heading: 'Decisioni simili passate', empty: 'Nessuna decisione passata comparabile.', + emptyDetail: 'Qui compariranno le decisioni su proposte comparabili.', details: { show: 'Mostra decisioni simili', + showEmpty: 'Mostra decisioni simili (nessuna trovata)', hide: 'Nascondi decisioni simili', }, verdict: { diff --git a/frontend/taskdeck-web/src/tests/views/paper/review/ReviewSimilarPast.spec.ts b/frontend/taskdeck-web/src/tests/views/paper/review/ReviewSimilarPast.spec.ts new file mode 100644 index 000000000..d126a3c51 --- /dev/null +++ b/frontend/taskdeck-web/src/tests/views/paper/review/ReviewSimilarPast.spec.ts @@ -0,0 +1,151 @@ +import { describe, expect, it } from 'vitest' +import { mount } from '@vue/test-utils' +import ReviewSimilarPast from '../../../../views/paper/review/ReviewSimilarPast.vue' +import type { SimilarPastRow } from '../../../../composables/usePaperReviewSelectors' + +/** + * ReviewSimilarPast — the card must tell the truth BEFORE the disclosure opens + * (#1940, the retained residual of #2166). + * + * The card shipped with the whole empty state locked inside a collapsed + * region: a reviewer looking at a proposal with no comparable history saw only + * "Show similar decisions" and had to open it to learn there was nothing to + * show. The fix keeps the disclosure in every state — PaperReviewView.spec.ts, + * PaperReviewView.language.spec.ts and the required E2E smoke all drive that + * button on an empty fixture — and hoists the fact above it. + * + * The empty sentence lives in exactly one place. Duplicating it above and + * inside the region would make the open state read as two separate findings. + */ + +const ROWS: SimilarPastRow[] = [ + { serial: '#PAST-1', title: 'A prior comparable decision', verdict: 'applied', date: '2026-08-20' }, + { serial: '#PAST-2', title: 'A prior rejected decision', verdict: 'rejected', date: '2026-08-19' }, +] + +const EMPTY_SENTENCE = 'No comparable past decisions.' + +function mountCard(rows: SimilarPastRow[]) { + return mount(ReviewSimilarPast, { + attachTo: document.body, + props: { + rows, + applyRate: + rows.length === 0 + ? { applied: 0, total: 0, ratio: 0 } + : { applied: 1, total: 2, ratio: 0.5 }, + }, + }) +} + +/** How many times a sentence appears in the rendered text of the whole card. */ +function occurrences(haystack: string, needle: string): number { + return haystack.split(needle).length - 1 +} + +describe('ReviewSimilarPast', () => { + describe('with no comparable decisions', () => { + it('states the emptiness at first paint, above the still-collapsed disclosure', () => { + const wrapper = mountCard([]) + const empty = wrapper.get('[data-testid="paper-review-similar-past-empty"]') + const details = wrapper.get('[data-testid="paper-review-similar-past-details"]') + + expect(empty.text()).toBe(EMPTY_SENTENCE) + expect(empty.isVisible()).toBe(true) + // The hoisted line is a sibling of the region, not a child of it: a + // child would still be invisible while the region is collapsed. + expect(details.find('[data-testid="paper-review-similar-past-empty"]').exists()).toBe(false) + expect(details.isVisible()).toBe(false) + + wrapper.unmount() + }) + + it('says so on the disclosure label too, so the closed control is not a promise', () => { + const wrapper = mountCard([]) + const button = wrapper.get('[data-testid="paper-review-similar-past-disclosure"]') + + expect(button.text()).toContain('Show similar decisions') + expect(button.text()).toContain('none found') + + wrapper.unmount() + }) + + it('keeps the disclosure present and correctly paired while collapsed', () => { + const wrapper = mountCard([]) + const button = wrapper.get('[data-testid="paper-review-similar-past-disclosure"]') + const details = wrapper.get('[data-testid="paper-review-similar-past-details"]') + + expect(button.element.tagName).toBe('BUTTON') + expect(button.attributes('type')).toBe('button') + expect(button.attributes('aria-expanded')).toBe('false') + expect(button.attributes('aria-controls')).toBe(details.attributes('id')) + expect(details.attributes('aria-labelledby')).toBe(button.attributes('id')) + expect(details.attributes('role')).toBe('region') + + wrapper.unmount() + }) + + it('never says the same thing twice', async () => { + const wrapper = mountCard([]) + const details = wrapper.get('[data-testid="paper-review-similar-past-details"]') + + expect(occurrences(wrapper.text(), EMPTY_SENTENCE)).toBe(1) + expect(details.text()).not.toContain(EMPTY_SENTENCE) + + await wrapper.get('[data-testid="paper-review-similar-past-disclosure"]').trigger('click') + + expect(occurrences(wrapper.text(), EMPTY_SENTENCE)).toBe(1) + expect(details.text()).not.toContain(EMPTY_SENTENCE) + + wrapper.unmount() + }) + + it('opens onto an explanation rather than onto nothing', async () => { + const wrapper = mountCard([]) + const button = wrapper.get('[data-testid="paper-review-similar-past-disclosure"]') + const details = wrapper.get('[data-testid="paper-review-similar-past-details"]') + + await button.trigger('click') + + expect(button.attributes('aria-expanded')).toBe('true') + expect(details.isVisible()).toBe(true) + // A region that opens to a zero-height void reads as a broken control, + // and the required E2E asserts this region is *visible* once opened. + expect(details.get('[data-testid="paper-review-similar-past-empty-detail"]').text()).toBe( + 'Decisions on comparable proposals will be listed here.', + ) + expect(wrapper.find('.paper-review-past__rate').exists()).toBe(false) + + wrapper.unmount() + }) + }) + + describe('with comparable decisions', () => { + it('keeps the rows and the apply-rate footer behind the disclosure', async () => { + const wrapper = mountCard(ROWS) + const button = wrapper.get('[data-testid="paper-review-similar-past-disclosure"]') + const details = wrapper.get('[data-testid="paper-review-similar-past-details"]') + const rate = wrapper.get('.paper-review-past__rate') + + expect(wrapper.find('[data-testid="paper-review-similar-past-empty"]').exists()).toBe(false) + expect(wrapper.text()).not.toContain(EMPTY_SENTENCE) + expect(button.text()).toContain('Show similar decisions') + expect(button.text()).not.toContain('none found') + expect(details.isVisible()).toBe(false) + expect(rate.isVisible()).toBe(false) + + await button.trigger('click') + + expect(details.isVisible()).toBe(true) + expect(details.text()).toContain('A prior comparable decision') + expect(details.text()).toContain('A prior rejected decision') + expect(rate.isVisible()).toBe(true) + expect(rate.text()).toContain('1 of 2 (50%)') + expect( + wrapper.find('[data-testid="paper-review-similar-past-empty-detail"]').exists(), + ).toBe(false) + + wrapper.unmount() + }) + }) +}) diff --git a/frontend/taskdeck-web/src/views/paper/review/ReviewSimilarPast.vue b/frontend/taskdeck-web/src/views/paper/review/ReviewSimilarPast.vue index 489e5c7c0..9910c8d15 100644 --- a/frontend/taskdeck-web/src/views/paper/review/ReviewSimilarPast.vue +++ b/frontend/taskdeck-web/src/views/paper/review/ReviewSimilarPast.vue @@ -1,5 +1,5 @@