From 7e9a573393f06711b281923473fa7eef9d23908f Mon Sep 17 00:00:00 2001 From: JOY <5027251+JOY@users.noreply.github.com> Date: Wed, 28 Oct 2026 20:01:30 +0700 Subject: [PATCH 1/4] fix(web): hide empty spec rows and groups on the product page The product page printed spec rows with no value as "-": a GPS watch's data speed, 4G bands and VoLTE, an earbud's FCC approval, and an earbud hardware card with a single row. The rows now come from buildProductSpecGroups, which leaves out rows with no value and groups with no row; group numbers and the badge count follow the shown groups. Compare leaves out a row that every column has empty, so "-" stays only beside a value. Refs #1186 Co-Authored-By: Claude Opus 5.5 --- apps/web/package.json | 2 +- .../app/[locale]/smartphones/[slug]/page.tsx | 620 +----------------- .../components/compare/CompareWorkspace.tsx | 37 +- .../lib/__tests__/product-spec-rows.test.ts | 118 ++++ .../src/lib/__tests__/spec-template.test.ts | 7 +- apps/web/src/lib/product-spec-rows.ts | 275 ++++++++ 6 files changed, 457 insertions(+), 602 deletions(-) create mode 100644 apps/web/src/lib/__tests__/product-spec-rows.test.ts create mode 100644 apps/web/src/lib/product-spec-rows.ts diff --git a/apps/web/package.json b/apps/web/package.json index cdcb9403..68ae5ccc 100644 --- a/apps/web/package.json +++ b/apps/web/package.json @@ -8,7 +8,7 @@ "lint": "eslint . --max-warnings 0", "preview": "next build && next start", "start": "next start", - "test": "pnpm --filter @repo/crawlers exec tsx --tsconfig ../web/tsconfig.test.json ../web/src/server/rate/__tests__/core-rate-architecture.test.ts && pnpm --filter @repo/crawlers exec tsx --tsconfig ../web/tsconfig.test.json ../web/src/app/api/v1/rate/__tests__/write-paths.test.ts && pnpm --filter @repo/crawlers exec tsx --tsconfig ../web/tsconfig.test.json ../web/src/app/api/v1/rate/__tests__/spec-scope.test.ts && pnpm --filter @repo/crawlers exec tsx --tsconfig ../web/tsconfig.test.json ../web/src/lib/__tests__/logger.test.ts && pnpm --filter @repo/crawlers exec tsx --tsconfig ../web/tsconfig.test.json ../web/src/lib/__tests__/review-display.test.ts && pnpm --filter @repo/crawlers exec tsx --tsconfig ../web/tsconfig.test.json ../web/src/lib/__tests__/concurrency.test.ts && pnpm --filter @repo/crawlers exec tsx --tsconfig ../web/tsconfig.test.json ../web/src/lib/__tests__/catalog-listing.test.ts && pnpm --filter @repo/crawlers exec tsx --tsconfig ../web/tsconfig.test.json ../web/src/lib/__tests__/discount-verdict-copy.test.ts && pnpm --filter @repo/crawlers exec tsx --tsconfig ../web/tsconfig.test.json ../web/src/lib/__tests__/product-detail-copy.test.ts && pnpm --filter @repo/crawlers exec tsx --tsconfig ../web/tsconfig.test.json ../web/src/lib/__tests__/product-competitors.test.ts && pnpm --filter @repo/crawlers exec tsx --tsconfig ../web/tsconfig.test.json ../web/src/lib/__tests__/analyzer-copy.test.ts && pnpm --filter @repo/crawlers exec tsx --tsconfig ../web/tsconfig.test.json ../web/src/lib/__tests__/sold-count.test.ts && pnpm --filter @repo/crawlers exec tsx --tsconfig ../web/tsconfig.test.json ../web/src/lib/__tests__/content-lang.test.ts && pnpm --filter @repo/crawlers exec tsx --tsconfig ../web/tsconfig.test.json ../web/src/lib/__tests__/image-url.test.ts && pnpm --filter @repo/crawlers exec tsx --tsconfig ../web/tsconfig.test.json ../web/src/lib/__tests__/client-messages.test.ts && pnpm --filter @repo/crawlers exec tsx --tsconfig ../web/tsconfig.test.json ../web/src/lib/__tests__/seo.test.ts && pnpm --filter @repo/crawlers exec tsx --tsconfig ../web/tsconfig.test.json ../web/src/lib/__tests__/offer-plausibility.test.ts && pnpm --filter @repo/crawlers exec tsx --tsconfig ../web/tsconfig.test.json ../web/src/lib/__tests__/product-market-stats.test.ts && pnpm --filter @repo/crawlers exec tsx --tsconfig ../web/tsconfig.test.json ../web/src/lib/__tests__/sitemap-lastmod.test.ts && pnpm --filter @repo/crawlers exec tsx --tsconfig ../web/tsconfig.test.json ../web/src/lib/__tests__/leaderboard-badge-copy.test.ts && pnpm --filter @repo/crawlers exec tsx --tsconfig ../web/tsconfig.test.json ../web/src/lib/__tests__/focus-trap.test.ts && pnpm --filter @repo/crawlers exec tsx --tsconfig ../web/tsconfig.test.json ../web/src/lib/__tests__/display-truth.test.ts && pnpm --filter @repo/crawlers exec tsx --tsconfig ../web/tsconfig.test.json ../web/src/lib/__tests__/spec-template.test.ts", + "test": "pnpm --filter @repo/crawlers exec tsx --tsconfig ../web/tsconfig.test.json ../web/src/server/rate/__tests__/core-rate-architecture.test.ts && pnpm --filter @repo/crawlers exec tsx --tsconfig ../web/tsconfig.test.json ../web/src/app/api/v1/rate/__tests__/write-paths.test.ts && pnpm --filter @repo/crawlers exec tsx --tsconfig ../web/tsconfig.test.json ../web/src/app/api/v1/rate/__tests__/spec-scope.test.ts && pnpm --filter @repo/crawlers exec tsx --tsconfig ../web/tsconfig.test.json ../web/src/lib/__tests__/logger.test.ts && pnpm --filter @repo/crawlers exec tsx --tsconfig ../web/tsconfig.test.json ../web/src/lib/__tests__/review-display.test.ts && pnpm --filter @repo/crawlers exec tsx --tsconfig ../web/tsconfig.test.json ../web/src/lib/__tests__/concurrency.test.ts && pnpm --filter @repo/crawlers exec tsx --tsconfig ../web/tsconfig.test.json ../web/src/lib/__tests__/catalog-listing.test.ts && pnpm --filter @repo/crawlers exec tsx --tsconfig ../web/tsconfig.test.json ../web/src/lib/__tests__/discount-verdict-copy.test.ts && pnpm --filter @repo/crawlers exec tsx --tsconfig ../web/tsconfig.test.json ../web/src/lib/__tests__/product-detail-copy.test.ts && pnpm --filter @repo/crawlers exec tsx --tsconfig ../web/tsconfig.test.json ../web/src/lib/__tests__/product-competitors.test.ts && pnpm --filter @repo/crawlers exec tsx --tsconfig ../web/tsconfig.test.json ../web/src/lib/__tests__/analyzer-copy.test.ts && pnpm --filter @repo/crawlers exec tsx --tsconfig ../web/tsconfig.test.json ../web/src/lib/__tests__/sold-count.test.ts && pnpm --filter @repo/crawlers exec tsx --tsconfig ../web/tsconfig.test.json ../web/src/lib/__tests__/content-lang.test.ts && pnpm --filter @repo/crawlers exec tsx --tsconfig ../web/tsconfig.test.json ../web/src/lib/__tests__/image-url.test.ts && pnpm --filter @repo/crawlers exec tsx --tsconfig ../web/tsconfig.test.json ../web/src/lib/__tests__/client-messages.test.ts && pnpm --filter @repo/crawlers exec tsx --tsconfig ../web/tsconfig.test.json ../web/src/lib/__tests__/seo.test.ts && pnpm --filter @repo/crawlers exec tsx --tsconfig ../web/tsconfig.test.json ../web/src/lib/__tests__/offer-plausibility.test.ts && pnpm --filter @repo/crawlers exec tsx --tsconfig ../web/tsconfig.test.json ../web/src/lib/__tests__/product-market-stats.test.ts && pnpm --filter @repo/crawlers exec tsx --tsconfig ../web/tsconfig.test.json ../web/src/lib/__tests__/sitemap-lastmod.test.ts && pnpm --filter @repo/crawlers exec tsx --tsconfig ../web/tsconfig.test.json ../web/src/lib/__tests__/leaderboard-badge-copy.test.ts && pnpm --filter @repo/crawlers exec tsx --tsconfig ../web/tsconfig.test.json ../web/src/lib/__tests__/focus-trap.test.ts && pnpm --filter @repo/crawlers exec tsx --tsconfig ../web/tsconfig.test.json ../web/src/lib/__tests__/display-truth.test.ts && pnpm --filter @repo/crawlers exec tsx --tsconfig ../web/tsconfig.test.json ../web/src/lib/__tests__/spec-template.test.ts && pnpm --filter @repo/crawlers exec tsx --tsconfig ../web/tsconfig.test.json ../web/src/lib/__tests__/product-spec-rows.test.ts", "typecheck": "tsc --noEmit" }, "browserslist": [ diff --git a/apps/web/src/app/[locale]/smartphones/[slug]/page.tsx b/apps/web/src/app/[locale]/smartphones/[slug]/page.tsx index 55d27fcc..a68d6bd0 100644 --- a/apps/web/src/app/[locale]/smartphones/[slug]/page.tsx +++ b/apps/web/src/app/[locale]/smartphones/[slug]/page.tsx @@ -1,5 +1,5 @@ import type { Metadata } from 'next'; -import { Fragment, type ReactNode } from 'react'; +import { Fragment } from 'react'; import { getTranslations, setRequestLocale } from 'next-intl/server'; import { Link } from '@/lib/navigation'; import { notFound } from 'next/navigation'; @@ -57,12 +57,12 @@ import type { KeySpecs, ProductSpecifications } from '@repo/shared-data'; import { normalizeRateLocale } from '@/lib/rate-i18n'; import { getSpecUiDictionary, - localizeSpecValue, specGroupHeading, specSectionBadge, } from '@/lib/spec-localizer'; import { contentLang } from '@/lib/content-lang'; import ContentText from '@/components/elementary/ContentText'; +import { buildProductSpecGroups } from '@/lib/product-spec-rows'; import { computeProductStandingInCategory, scoreMasterProductForLeaderboard, @@ -76,8 +76,6 @@ import ClientMessages from '@/components/providers/ClientMessages'; import { applyCategorySpecTemplate, getCategorySpecTemplate, - specFieldApplies, - type SpecGroupKey, } from '@repo/shared-data/rules'; import { isOfferUnavailable, @@ -963,583 +961,16 @@ export async function ProductDetailView({ {/* SECTION #2: Spec groups, per category template (DOS/Rate#1142) */} {(() => { const specUi = getSpecUiDictionary(locale); - const loc = (v: string | number | boolean | null | undefined) => ( - - ); - const groupHeading = (group: SpecGroupKey) => - specGroupHeading( - specUi, - group, - specTemplate.groups.indexOf(group) + 1, - specTemplate.has5g - ); - const shows = (group: SpecGroupKey, field: string) => - specFieldApplies(specTemplate, group, field); - const groupNodes: Record = { - display: ( -
-
-

- {groupHeading('display')} -

-
-
- {specUi.fields.screenSize} -
-
- {specs?.display?.size_inch - ? `${specs.display.size_inch} inch` - : product.screen_size_inch - ? `${product.screen_size_inch} inch` - : '-'} -
-
- {specUi.fields.panelType} -
-
- {loc(specs?.display?.type || keySpecs?.screen)} -
-
- {specUi.fields.resolution} -
-
- {loc(specs?.display?.resolution)} -
-
- {specUi.fields.refreshRate} -
-
- {specs?.display?.refresh_rate_hz - ? `${specs.display.refresh_rate_hz} Hz` - : '-'} -
-
{specUi.fields.ppi}
-
- {specs?.display?.ppi ? `${specs.display.ppi} ppi` : '-'} -
-
- {specUi.fields.protection} -
-
- {loc(specs?.display?.protection)} -
-
- {specUi.fields.displayFeatures} -
-
- {loc(specs?.display?.features)} -
-
-
-
- ), - hardware: ( -
-
-

- {groupHeading('hardware')} -

-
-
- {specUi.fields.chipset} -
-
- {loc(specs?.hardware?.chipset || keySpecs?.chip)} -
- {shows('hardware', 'cpu') && ( - <> -
- {specUi.fields.cpu} -
-
- {loc(specs?.hardware?.cpu)} -
- - )} - {shows('hardware', 'gpu') && ( - <> -
- {specUi.fields.gpu} -
-
- {loc(specs?.hardware?.gpu)} -
- - )} - {shows('hardware', 'ram_gb') && ( - <> -
- {specUi.fields.ram} -
-
- {specs?.hardware?.ram_gb - ? `${specs.hardware.ram_gb} GB` - : product.ram_gb - ? `${product.ram_gb} GB` - : '-'} -
- - )} - {shows('hardware', 'storage_gb') && ( - <> -
- {specUi.fields.storage} -
-
- {specs?.hardware?.storage_gb - ? `${specs.hardware.storage_gb} GB` - : product.storage_gb - ? `${product.storage_gb} GB` - : '-'} -
- - )} - {shows('hardware', 'storage_expansion') && ( - <> -
- {specUi.fields.storageExpansion} -
-
- {loc(specs?.hardware?.storage_expansion)} -
- - )} - {shows('hardware', 'os') && ( - <> -
- {specUi.fields.os} -
-
- {loc(specs?.hardware?.os)} -
- - )} -
-
-
- ), - camera: ( -
-
-

- {groupHeading('camera')} -

-
-
- {specUi.fields.rearSetup} -
-
- {loc(specs?.camera?.rear_setup)} -
-
- {specUi.fields.mainCamera} -
-
- {loc(specs?.camera?.main_camera || keySpecs?.camera)} -
-
- {specUi.fields.secondCamera} -
-
- {loc(specs?.camera?.second_camera)} -
-
- {specUi.fields.thirdCamera} -
-
- {loc(specs?.camera?.third_camera)} -
- {specs?.camera?.fourth_camera && ( - <> -
- {specUi.fields.fourthCamera} -
-
- {loc(specs.camera.fourth_camera)} -
- - )} -
- {specUi.fields.frontCamera} -
-
- {loc(specs?.camera?.front_camera)} -
-
- {specUi.fields.videoAndFlash} -
-
- {loc( - [specs?.camera?.video_recording, specs?.camera?.flash] - .filter(Boolean) - .join(' | ') - )} -
-
-
-
- ), - battery: ( -
-
-

- {groupHeading('battery')} -

-
-
- {specUi.fields.batteryCapacity} -
-
- {specs?.battery?.capacity_mah - ? `${specs.battery.capacity_mah} mAh` - : product.battery_mah - ? `${product.battery_mah} mAh` - : loc(keySpecs?.battery)} -
-
- {specUi.fields.batteryType} -
-
- {loc(specs?.battery?.type)} -
-
- {specUi.fields.chargingTech} -
-
- {loc(specs?.battery?.charging)} -
-
- {specUi.fields.fastCharging} -
-
- {specs?.battery?.fast_charging_w - ? `${specs.battery.fast_charging_w} W` - : '-'} -
-
- {specUi.fields.reverseCharging} -
-
- {typeof specs?.battery?.reverse_charging === 'string' - ? loc(specs.battery.reverse_charging) - : specs?.battery?.reverse_charging === true - ? specUi.supported - : specs?.battery?.reverse_charging === false - ? specUi.no - : '-'} -
-
-
-
- ), - design: ( -
-
-

- {groupHeading('design')} -

-
-
- {specUi.fields.dimensions} -
-
- {loc(specs?.design?.dimensions)} -
-
- {specUi.fields.weight} -
-
- {specs?.design?.weight_g - ? `${specs.design.weight_g} g` - : '-'} -
-
- {specUi.fields.materialsAndBuild} -
-
- {loc( - [specs?.design?.materials, specs?.design?.build] - .filter(Boolean) - .join(' | ') - )} -
-
- {specUi.fields.resistance} -
-
- {loc(specs?.design?.resistance)} -
- {shows('design', 'biometrics') && ( - <> -
- {specUi.fields.biometrics} -
-
- {loc(specs?.design?.biometrics)} -
- - )} -
- {specUi.fields.physicalKeys} -
-
- {loc(specs?.design?.keys)} -
-
-
-
- ), - cellular: ( -
-
-

- {groupHeading('cellular')} -

-
- {shows('cellular', 'has_5g') && ( - <> -
- {specUi.fields.support5g} -
-
- {(specs?.cellular?.has_5g ?? product.has_5g) - ? specUi.has5gYes - : specUi.no} -
- - )} -
- {specUi.fields.networkTech} -
-
- {loc(specs?.cellular?.technology)} -
-
- {specUi.fields.simType} -
-
- {loc(specs?.cellular?.sim_type)} -
-
- {specUi.fields.dataSpeed} -
-
- {loc(specs?.cellular?.data_speed)} -
-
- {shows('cellular', 'bands_5g') - ? specUi.fields.bands5g4g - : specUi.fields.bands4g} -
-
- {loc( - [ - shows('cellular', 'bands_5g') - ? specs?.cellular?.bands_5g - : null, - specs?.cellular?.bands_4g, - ] - .filter(Boolean) - .join(' | ') - )} -
-
- {specUi.fields.volteVowifi} -
-
- {specs?.cellular?.volte || specs?.cellular?.vowifi - ? [ - specs?.cellular?.volte ? 'VoLTE' : null, - specs?.cellular?.vowifi ? 'VoWiFi' : null, - ] - .filter(Boolean) - .join(' + ') - : '-'} -
-
-
-
- ), - connectivity: ( -
-
-

- {groupHeading('connectivity')} -

-
- {shows('connectivity', 'wlan') && ( - <> -
- {specUi.fields.wlan} -
-
- {loc(specs?.connectivity?.wlan)} -
- - )} -
- {specUi.fields.bluetooth} -
-
- {loc(specs?.connectivity?.bluetooth)} -
-
- {specUi.fields.usbPort} -
-
- {loc(specs?.connectivity?.usb)} -
- {shows('connectivity', 'nfc') && ( - <> -
- {specUi.fields.nfc} -
-
- {specs?.connectivity?.nfc === true - ? specUi.yes - : specs?.connectivity?.nfc === false - ? specUi.no - : '-'} -
- - )} - {shows('connectivity', 'location') && ( - <> -
- {specUi.fields.locationGps} -
-
- {loc(specs?.connectivity?.location)} -
- - )} -
- {specUi.fields.sensorsAndOther} -
-
- {loc( - [ - specs?.connectivity?.sensors, - specs?.connectivity?.other, - ] - .filter(Boolean) - .join(' | ') - )} -
-
-
-
- ), - multimedia: ( -
-
-

- {groupHeading('multimedia')} -

-
-
- {specUi.fields.loudspeaker} -
-
- {loc(specs?.multimedia?.loudspeaker)} -
- {shows('multimedia', 'headphone_jack_35mm') && ( - <> -
- {specUi.fields.jack35mm} -
-
- {specs?.multimedia?.headphone_jack_35mm === true - ? specUi.yes - : specs?.multimedia?.headphone_jack_35mm === false - ? specUi.no - : '-'} -
- - )} -
- {specUi.fields.audioFeatures} -
-
- {loc(specs?.multimedia?.audio_features)} -
-
-
-
- ), - availability: ( -
-
-

- {groupHeading('availability')} -

-
-
- {specUi.fields.announcedDate} -
-
- {loc(specs?.availability?.announced)} -
-
- {specUi.fields.commercialStatus} -
-
- {specs?.availability?.status - ? loc(specs.availability.status) - : specUi.availableStatus} -
-
- {specUi.fields.msrpPrice} -
-
- {loc(specs?.availability?.prices_msrp)} -
-
-
-
- ), - regulatory: ( -
-
-

- {groupHeading('regulatory')} -

-
-
- {specUi.fields.certifications} -
-
- {loc(specs?.regulatory?.certifications)} -
-
- {specUi.fields.fccApproval} -
-
- {loc(specs?.regulatory?.fcc_approval)} -
-
- {specUi.fields.sarLevel} -
-
- {[ - specs?.regulatory?.sar_head - ? `${specUi.fields.sarHeadPrefix}: ${specs.regulatory.sar_head}` - : null, - specs?.regulatory?.sar_body - ? `${specUi.fields.sarBodyPrefix}: ${specs.regulatory.sar_body}` - : null, - ] - .filter(Boolean) - .join(' | ') || '-'} -
-
-
-
- ), - }; + // Rows with no value and groups with no row are left out + // (DOS/Rate#1186). + const specGroups = buildProductSpecGroups({ + specs, + keySpecs, + product, + template: specTemplate, + ui: specUi, + locale, + }); return (
- {specSectionBadge(specUi, specTemplate.groups.length)} + {specSectionBadge(specUi, specGroups.length)}
@@ -1577,8 +1008,29 @@ export async function ProductDetailView({ showLessLabel={t('specsShowLess')} openOnHash="#specs-section" > - {specTemplate.groups.map((group) => ( - {groupNodes[group]} + {specGroups.map(({ group, rows }, i) => ( +
+
+

+ {specGroupHeading( + specUi, + group, + i + 1, + specTemplate.has5g + )} +

+
+ {rows.map(({ label, value }) => ( + +
{label}
+
+ +
+ + ))} +
+
+
))} diff --git a/apps/web/src/components/compare/CompareWorkspace.tsx b/apps/web/src/components/compare/CompareWorkspace.tsx index 01d8e9eb..98305237 100644 --- a/apps/web/src/components/compare/CompareWorkspace.tsx +++ b/apps/web/src/components/compare/CompareWorkspace.tsx @@ -67,6 +67,7 @@ import { localizeSpecValue, specGroupHeading, } from '@/lib/spec-localizer'; +import { compareRowIsShown } from '@/lib/product-spec-rows'; import ContentText from '@/components/elementary/ContentText'; import { productImageUrl } from '@/lib/rate-domain-core'; @@ -862,6 +863,24 @@ export default function CompareWorkspace({ [allSpecGroups, activeCategory] ); + // No row that every column leaves empty, and with `showDiffOnly` no row + // where every column agrees (DOS/Rate#1186). + const visibleSpecGroups = specGroups + .map((group) => ({ + ...group, + rows: group.rows.filter((row) => + compareRowIsShown( + products.map((p) => row.renderValue(p)), + showDiffOnly + ) + ), + })) + .filter((group) => group.rows.length > 0); + // Spec groups are numbered from 1 after the market group. + const numberedSpecGroups = visibleSpecGroups.filter( + (group) => group.id !== 'market_trust' + ); + const searchInputRef = useRef(null); const searchBoxRef = useRef(null); const restoredTrayRef = useRef(false); @@ -2257,20 +2276,8 @@ export default function CompareWorkspace({ - )} - {specGroups.map((group, groupIndex) => { - // Filter rows when `showDiffOnly` is enabled - const visibleRows = group.rows.filter((row) => { - if (!showDiffOnly || products.length < 2) return true; - const rendered = products.map((p) => - row.renderValue(p).trim() - ); - const uniqueVals = new Set(rendered); - return uniqueVals.size > 1; - }); - - if (visibleRows.length === 0) { - return null; - } + {visibleSpecGroups.map((group) => { + const visibleRows = group.rows; const totalCols = products.length + @@ -2285,7 +2292,7 @@ export default function CompareWorkspace({ ? specGroupHeading( specDict, group.id as SpecGroupKey, - groupIndex, + numberedSpecGroups.indexOf(group) + 1, getCategorySpecTemplate(activeCategory).has5g ) : group.title; diff --git a/apps/web/src/lib/__tests__/product-spec-rows.test.ts b/apps/web/src/lib/__tests__/product-spec-rows.test.ts new file mode 100644 index 00000000..65b6adc1 --- /dev/null +++ b/apps/web/src/lib/__tests__/product-spec-rows.test.ts @@ -0,0 +1,118 @@ +import assert from 'node:assert/strict'; +import { getCategorySpecTemplate } from '@repo/shared-data/rules'; +import type { KeySpecs, ProductSpecifications } from '@repo/shared-data'; +import { + buildProductSpecGroups, + compareRowIsShown, + isEmptySpecValue, +} from '../product-spec-rows'; +import { getSpecUiDictionary } from '../spec-localizer'; + +const groupsFor = ( + category: string, + specs: Record, + keySpecs: Record = {}, + product: Record = {} +) => + buildProductSpecGroups({ + specs: specs as ProductSpecifications, + keySpecs: keySpecs as KeySpecs, + product, + template: getCategorySpecTemplate(category), + ui: getSpecUiDictionary('vi'), + locale: 'vi', + }); + +const labelsOf = (groups: ReturnType, group: string) => + groups.find((g) => g.group === group)?.rows.map((r) => r.label) ?? null; + +function testProductPage() { + const ui = getSpecUiDictionary('vi'); + + // A GPS watch: no cellular value at all, so no cellular group; empty rows + // of other groups are left out, filled ones kept. + const watch = groupsFor('smartwatches', { + display: { size_inch: 1.9, type: 'LTPO OLED', resolution: '' }, + hardware: { chipset: 'Apple S11' }, + battery: { capacity_mah: 327 }, + cellular: { technology: '', data_speed: null, bands_4g: '' }, + connectivity: { bluetooth: '5.3' }, + }); + assert.equal(labelsOf(watch, 'cellular'), null); + assert.deepEqual(labelsOf(watch, 'display'), [ + ui.fields.screenSize, + ui.fields.panelType, + ]); + assert.deepEqual(labelsOf(watch, 'hardware'), [ui.fields.chipset]); + // Availability keeps its default commercial status. + assert.deepEqual(labelsOf(watch, 'availability'), [ + ui.fields.commercialStatus, + ]); + for (const { rows } of watch) { + for (const { value } of rows) { + assert.ok(!isEmptySpecValue(value), `empty row: ${value}`); + } + } + + // Earbuds: an empty FCC row is gone, certifications stay; the hardware + // card keeps only the audio chip. + const buds = groupsFor( + 'earbuds', + { + hardware: { chipset: 'H2' }, + regulatory: { certifications: 'CE', fcc_approval: '-' }, + multimedia: { audio_features: 'ANC' }, + }, + { battery: '6 giờ' } + ); + assert.deepEqual(labelsOf(buds, 'regulatory'), [ui.fields.certifications]); + assert.deepEqual(labelsOf(buds, 'hardware'), [ui.fields.chipset]); + assert.deepEqual( + buds.map((g) => g.group), + ['multimedia', 'battery', 'hardware', 'availability', 'regulatory'] + ); + + // A group with nothing but empty values disappears. + const bare = groupsFor('earbuds', { + design: { dimensions: '', weight_g: 0, keys: null }, + }); + assert.equal(labelsOf(bare, 'design'), null); + + // Phones keep their filled rows and flat-column fallbacks. + const phone = groupsFor( + 'smartphones', + { hardware: { chipset: 'A18' } }, + {}, + { ram_gb: 8, storage_gb: 128, has_5g: true } + ); + assert.deepEqual( + phone.find((g) => g.group === 'hardware')?.rows.map((r) => r.value), + ['A18', '8 GB', '128 GB'] + ); + assert.equal( + phone.find((g) => g.group === 'cellular')?.rows[0]?.value, + ui.has5gYes + ); + console.warn(' ✅ Product page leaves out empty rows and groups'); +} + +function testCompare() { + assert.equal(compareRowIsShown(['-', '-'], false), false); + assert.equal(compareRowIsShown(['', ' - '], true), false); + // "-" stays where another column has a value. + assert.equal(compareRowIsShown(['-', 'LTE'], false), true); + assert.equal(compareRowIsShown(['-', 'LTE'], true), true); + assert.equal(compareRowIsShown(['LTE', 'LTE'], true), false); + assert.equal(compareRowIsShown(['LTE', 'LTE'], false), true); + assert.equal(compareRowIsShown(['LTE'], true), true); + assert.equal(compareRowIsShown(['-'], false), false); + console.warn(' ✅ Compare keeps "-" only beside a value'); +} + +function main() { + console.warn('🧪 Product spec rows suite (DOS/Rate#1186)'); + testProductPage(); + testCompare(); +} + +main(); diff --git a/apps/web/src/lib/__tests__/spec-template.test.ts b/apps/web/src/lib/__tests__/spec-template.test.ts index 94aa3dba..763772f3 100644 --- a/apps/web/src/lib/__tests__/spec-template.test.ts +++ b/apps/web/src/lib/__tests__/spec-template.test.ts @@ -216,7 +216,7 @@ async function testSurfaces() { // Compare headings come from the template too: no "5G" heading for a watch. assert.match( compare, - /specGroupHeading\(\s*specDict,\s*group\.id as SpecGroupKey,\s*groupIndex,\s*getCategorySpecTemplate\(activeCategory\)\.has5g\s*\)/ + /specGroupHeading\(\s*specDict,\s*group\.id as SpecGroupKey,\s*numberedSpecGroups\.indexOf\(group\) \+ 1,\s*getCategorySpecTemplate\(activeCategory\)\.has5g\s*\)/ ); const detail = readSource('../../app/[locale]/smartphones/[slug]/page.tsx'); assert.match(detail, /categorySlug=\{categorySlug\}/); @@ -304,7 +304,10 @@ async function main() { /const product = applyCategorySpecTemplate\(categorySlug, storedProduct\)/ ); assert.match(detail, /applyCategorySpecTemplate\(categorySlug, peer\)/); - assert.match(detail, /specTemplate\.groups\.map\(/); + assert.match( + detail, + /buildProductSpecGroups\(\{[^}]*template: specTemplate,/ + ); assert.doesNotMatch(detail, /specUi\.groups\.\w+\}/); const catalog = readSource('../../app/[locale]/smartphones/page.tsx'); assert.match( diff --git a/apps/web/src/lib/product-spec-rows.ts b/apps/web/src/lib/product-spec-rows.ts new file mode 100644 index 00000000..119aee69 --- /dev/null +++ b/apps/web/src/lib/product-spec-rows.ts @@ -0,0 +1,275 @@ +import type { KeySpecs, ProductSpecifications } from '@repo/shared-data'; +import { + specFieldApplies, + type CategorySpecTemplate, + type SpecGroupKey, +} from '@repo/shared-data/rules'; +import { localizeSpecValue, type SpecUiDictionary } from './spec-localizer'; + +export interface ProductSpecRow { + label: string; + value: string; +} + +export interface ProductSpecGroup { + group: SpecGroupKey; + rows: ProductSpecRow[]; +} + +/** The flat columns the spec section falls back to. */ +export interface ProductSpecFallbacks { + screen_size_inch?: number | null; + ram_gb?: number | null; + storage_gb?: number | null; + battery_mah?: number | null; + has_5g?: boolean | null; +} + +/** A value the page has nothing to show for: empty or a lone dash. */ +export function isEmptySpecValue(value: string | null | undefined): boolean { + const text = (value ?? '').trim(); + return text === '' || text === '-'; +} + +/** + * Whether compare shows a row (DOS/Rate#1186): not when no compared product + * has a value, so "-" stays only where another column has one. With + * `diffOnly`, rows where every column agrees are left out too. + */ +export function compareRowIsShown( + values: readonly string[], + diffOnly: boolean +): boolean { + if (values.length === 0) return true; + if (values.every(isEmptySpecValue)) return false; + if (!diffOnly || values.length < 2) return true; + return new Set(values.map((v) => v.trim())).size > 1; +} + +const joined = (parts: unknown[], separator: string): string => + parts.filter(Boolean).join(separator); + +/** + * The product page's spec groups in the category's template order, with the + * rows that have no value left out and a group with no row left out + * (DOS/Rate#1186). The page used to print those rows as "-". + */ +export function buildProductSpecGroups({ + specs, + keySpecs, + product, + template, + ui, + locale, +}: { + specs: ProductSpecifications | null | undefined; + keySpecs: KeySpecs | null | undefined; + product: ProductSpecFallbacks; + template: CategorySpecTemplate; + ui: SpecUiDictionary; + locale: string; +}): ProductSpecGroup[] { + const s = specs ?? ({} as ProductSpecifications); + const k = keySpecs ?? ({} as KeySpecs); + const f = ui.fields; + const loc = (v: string | number | boolean | null | undefined) => + localizeSpecValue(v, locale); + const shows = (group: SpecGroupKey, field: string) => + specFieldApplies(template, group, field); + const withUnit = (value: unknown, unit: string) => + value ? `${value} ${unit}` : ''; + const yesNo = (value: unknown, yes = ui.yes) => + value === true ? yes : value === false ? ui.no : ''; + const showBands5g = shows('cellular', 'bands_5g'); + + const groups: Record = { + display: [ + { + label: f.screenSize, + value: + withUnit(s.display?.size_inch, 'inch') || + withUnit(product.screen_size_inch, 'inch'), + }, + { label: f.panelType, value: loc(s.display?.type || k.screen) }, + { label: f.resolution, value: loc(s.display?.resolution) }, + { + label: f.refreshRate, + value: withUnit(s.display?.refresh_rate_hz, 'Hz'), + }, + { label: f.ppi, value: withUnit(s.display?.ppi, 'ppi') }, + { label: f.protection, value: loc(s.display?.protection) }, + { label: f.displayFeatures, value: loc(s.display?.features) }, + ], + hardware: [ + { label: f.chipset, value: loc(s.hardware?.chipset || k.chip) }, + shows('hardware', 'cpu') && { label: f.cpu, value: loc(s.hardware?.cpu) }, + shows('hardware', 'gpu') && { label: f.gpu, value: loc(s.hardware?.gpu) }, + shows('hardware', 'ram_gb') && { + label: f.ram, + value: + withUnit(s.hardware?.ram_gb, 'GB') || withUnit(product.ram_gb, 'GB'), + }, + shows('hardware', 'storage_gb') && { + label: f.storage, + value: + withUnit(s.hardware?.storage_gb, 'GB') || + withUnit(product.storage_gb, 'GB'), + }, + shows('hardware', 'storage_expansion') && { + label: f.storageExpansion, + value: loc(s.hardware?.storage_expansion), + }, + shows('hardware', 'os') && { label: f.os, value: loc(s.hardware?.os) }, + ], + camera: [ + { label: f.rearSetup, value: loc(s.camera?.rear_setup) }, + { + label: f.mainCamera, + value: loc(s.camera?.main_camera || k.camera), + }, + { label: f.secondCamera, value: loc(s.camera?.second_camera) }, + { label: f.thirdCamera, value: loc(s.camera?.third_camera) }, + { label: f.fourthCamera, value: loc(s.camera?.fourth_camera) }, + { label: f.frontCamera, value: loc(s.camera?.front_camera) }, + { + label: f.videoAndFlash, + value: loc(joined([s.camera?.video_recording, s.camera?.flash], ' | ')), + }, + ], + battery: [ + { + label: f.batteryCapacity, + value: + withUnit(s.battery?.capacity_mah, 'mAh') || + withUnit(product.battery_mah, 'mAh') || + loc(k.battery), + }, + { label: f.batteryType, value: loc(s.battery?.type) }, + { label: f.chargingTech, value: loc(s.battery?.charging) }, + { + label: f.fastCharging, + value: withUnit(s.battery?.fast_charging_w, 'W'), + }, + { + label: f.reverseCharging, + value: + typeof s.battery?.reverse_charging === 'string' + ? loc(s.battery.reverse_charging) + : yesNo(s.battery?.reverse_charging, ui.supported), + }, + ], + design: [ + { label: f.dimensions, value: loc(s.design?.dimensions) }, + { label: f.weight, value: withUnit(s.design?.weight_g, 'g') }, + { + label: f.materialsAndBuild, + value: loc(joined([s.design?.materials, s.design?.build], ' | ')), + }, + { label: f.resistance, value: loc(s.design?.resistance) }, + shows('design', 'biometrics') && { + label: f.biometrics, + value: loc(s.design?.biometrics), + }, + { label: f.physicalKeys, value: loc(s.design?.keys) }, + ], + cellular: [ + shows('cellular', 'has_5g') && { + label: f.support5g, + value: (s.cellular?.has_5g ?? product.has_5g) ? ui.has5gYes : ui.no, + }, + { label: f.networkTech, value: loc(s.cellular?.technology) }, + { label: f.simType, value: loc(s.cellular?.sim_type) }, + { label: f.dataSpeed, value: loc(s.cellular?.data_speed) }, + { + label: showBands5g ? f.bands5g4g : f.bands4g, + value: loc( + joined( + [showBands5g ? s.cellular?.bands_5g : null, s.cellular?.bands_4g], + ' | ' + ) + ), + }, + { + label: f.volteVowifi, + value: joined( + [ + s.cellular?.volte ? 'VoLTE' : null, + s.cellular?.vowifi ? 'VoWiFi' : null, + ], + ' + ' + ), + }, + ], + connectivity: [ + shows('connectivity', 'wlan') && { + label: f.wlan, + value: loc(s.connectivity?.wlan), + }, + { label: f.bluetooth, value: loc(s.connectivity?.bluetooth) }, + { label: f.usbPort, value: loc(s.connectivity?.usb) }, + shows('connectivity', 'nfc') && { + label: f.nfc, + value: yesNo(s.connectivity?.nfc), + }, + shows('connectivity', 'location') && { + label: f.locationGps, + value: loc(s.connectivity?.location), + }, + { + label: f.sensorsAndOther, + value: loc( + joined([s.connectivity?.sensors, s.connectivity?.other], ' | ') + ), + }, + ], + multimedia: [ + { label: f.loudspeaker, value: loc(s.multimedia?.loudspeaker) }, + shows('multimedia', 'headphone_jack_35mm') && { + label: f.jack35mm, + value: yesNo(s.multimedia?.headphone_jack_35mm), + }, + { label: f.audioFeatures, value: loc(s.multimedia?.audio_features) }, + ], + availability: [ + { label: f.announcedDate, value: loc(s.availability?.announced) }, + { + label: f.commercialStatus, + value: s.availability?.status + ? loc(s.availability.status) + : ui.availableStatus, + }, + { label: f.msrpPrice, value: loc(s.availability?.prices_msrp) }, + ], + regulatory: [ + { + label: f.certifications, + value: loc(s.regulatory?.certifications), + }, + { label: f.fccApproval, value: loc(s.regulatory?.fcc_approval) }, + { + label: f.sarLevel, + value: joined( + [ + s.regulatory?.sar_head + ? `${f.sarHeadPrefix}: ${s.regulatory.sar_head}` + : null, + s.regulatory?.sar_body + ? `${f.sarBodyPrefix}: ${s.regulatory.sar_body}` + : null, + ], + ' | ' + ), + }, + ], + }; + + return template.groups + .map((group) => ({ + group, + rows: groups[group].filter( + (row): row is ProductSpecRow => + row !== false && !isEmptySpecValue(row.value) + ), + })) + .filter(({ rows }) => rows.length > 0); +} From 7b2e9941fc2e6c456b1f3295959fd57b8c425bd9 Mon Sep 17 00:00:00 2001 From: JOY <5027251+JOY@users.noreply.github.com> Date: Wed, 28 Oct 2026 20:03:58 +0700 Subject: [PATCH 2/4] fix(web): compare hides rows by the localized cell text Review of the previous commit: compare checked the raw value, so a row whose every cell localizes "N/A" to "-" stayed. The JSON export now skips all-empty rows like the CSV export. Refs #1186 Co-Authored-By: Claude Opus 5.5 --- .../components/compare/CompareWorkspace.tsx | 25 +++++++++++++------ 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/apps/web/src/components/compare/CompareWorkspace.tsx b/apps/web/src/components/compare/CompareWorkspace.tsx index 98305237..3aa09adb 100644 --- a/apps/web/src/components/compare/CompareWorkspace.tsx +++ b/apps/web/src/components/compare/CompareWorkspace.tsx @@ -870,7 +870,8 @@ export default function CompareWorkspace({ ...group, rows: group.rows.filter((row) => compareRowIsShown( - products.map((p) => row.renderValue(p)), + // The cells show the localized text, where "N/A" becomes "-". + products.map((p) => localizeSpecValue(row.renderValue(p), locale)), showDiffOnly ) ), @@ -1293,13 +1294,21 @@ export default function CompareWorkspace({ matrix: specGroups.map((group) => ({ groupId: group.id, groupTitle: group.title, - rows: group.rows.map((row) => ({ - id: row.id, - label: row.label, - values: Object.fromEntries( - products.map((p) => [p.slug, row.renderValue(p)]) - ), - })), + // Like the CSV, no row that every product leaves empty. + rows: group.rows + .filter((row) => + compareRowIsShown( + products.map((p) => row.renderValue(p)), + false + ) + ) + .map((row) => ({ + id: row.id, + label: row.label, + values: Object.fromEntries( + products.map((p) => [p.slug, row.renderValue(p)]) + ), + })), })), }; const blob = new Blob([JSON.stringify(payload, null, 2)], { From 3b6cba07ff1339af1b4b8d6779ae93e6e3873452 Mon Sep 17 00:00:00 2001 From: JOY <5027251+JOY@users.noreply.github.com> Date: Wed, 28 Oct 2026 20:12:26 +0700 Subject: [PATCH 3/4] test(web): e2e expects hidden empty spec groups The seeded earbud has values only for audio, battery, Bluetooth and the sale status, and the seeded phone read as a watch has no network value once 5G is dropped, so #1186 now hides their empty groups. The 4G-only bands label moves to product-spec-rows.test.ts. Refs #1186 Co-Authored-By: Claude Opus 5.5 --- apps/web/e2e/category-spec-template.spec.ts | 15 +++++++++++---- .../src/lib/__tests__/product-spec-rows.test.ts | 7 +++++++ 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/apps/web/e2e/category-spec-template.spec.ts b/apps/web/e2e/category-spec-template.spec.ts index bf811d4e..6839b797 100644 --- a/apps/web/e2e/category-spec-template.spec.ts +++ b/apps/web/e2e/category-spec-template.spec.ts @@ -11,7 +11,10 @@ test('earbud page shows audio specs and no phone fields', async ({ page }) => { const specs = page.locator('#specs-section'); const headings = specs.locator('h3'); await expect(headings.first()).toHaveText(/^1\. Âm thanh/); - await expect(headings).toHaveCount(7); + // Groups with no value are hidden (DOS/Rate#1186): the seeded bud has + // audio, battery, Bluetooth and the default sale status, nothing else. + await expect(headings).toHaveCount(4); + await expect(headings.nth(1)).toHaveText(/^2\. Pin/); await expect(specs).toContainText('E2E 10.5mm driver'); await expect(specs).toContainText('E2E ANC'); for (const text of [ @@ -24,6 +27,8 @@ test('earbud page shows audio specs and no phone fields', async ({ page }) => { 'E2E Mic Array', 'E2E Audio Engine', '5G', + 'Phần cứng', + 'Pháp lý', ]) { await expect(specs).not.toContainText(text); } @@ -53,9 +58,11 @@ test('a watch page never shows 5G', async ({ page }) => { await expect(meta).toContainText('Apple'); await expect(meta).not.toContainText('5G'); const specs = page.locator('#specs-section'); - await expect(specs).toContainText('Mạng Di động (Cellular)'); - // The bands row stays, as 4G only. - await expect(specs).toContainText('Băng tần 4G'); + await expect(specs).toContainText('Màn hình (Display)'); + // The seeded phone has no network value left once 5G is dropped, so the + // cellular group is hidden (DOS/Rate#1186); the 4G-only bands label is + // covered by product-spec-rows.test.ts. + await expect(specs).not.toContainText('Mạng Di động'); await expect(specs).not.toContainText('5G'); await expect(specs).not.toContainText('Camera'); }); diff --git a/apps/web/src/lib/__tests__/product-spec-rows.test.ts b/apps/web/src/lib/__tests__/product-spec-rows.test.ts index 65b6adc1..4df84d15 100644 --- a/apps/web/src/lib/__tests__/product-spec-rows.test.ts +++ b/apps/web/src/lib/__tests__/product-spec-rows.test.ts @@ -48,6 +48,13 @@ function testProductPage() { assert.deepEqual(labelsOf(watch, 'availability'), [ ui.fields.commercialStatus, ]); + // A cellular watch keeps its 4G bands under the 4G-only label. + const lte = groupsFor('smartwatches', { + cellular: { has_5g: true, bands_4g: 'B1/B3', bands_5g: 'n78' }, + }); + assert.deepEqual(lte.find((g) => g.group === 'cellular')?.rows, [ + { label: ui.fields.bands4g, value: 'B1/B3' }, + ]); for (const { rows } of watch) { for (const { value } of rows) { assert.ok(!isEmptySpecValue(value), `empty row: ${value}`); From 0029882de56370253f8f2d0beb192d10fe315f45 Mon Sep 17 00:00:00 2001 From: JOY <5027251+JOY@users.noreply.github.com> Date: Wed, 28 Oct 2026 20:18:32 +0700 Subject: [PATCH 4/4] fix(web): no spec toggle without hidden groups; e2e measures the hidden part Hiding empty groups (#1186) left the phone toggle on products with three groups or fewer, where it opens nothing; PhoneCollapsible now takes `collapsible` and the spec section passes it only past three groups. The collapse e2e asserted collapsed < 60% of expanded, which held for ten full groups but not for the sparse seeded phone, now five short groups. It now checks that collapsing removes the height of the groups it hides. Refs #1186 Co-Authored-By: Claude Opus 5.5 --- apps/web/e2e/product-specs-collapse.spec.ts | 13 +++++++++++-- .../src/app/[locale]/smartphones/[slug]/page.tsx | 1 + .../web/src/components/product/PhoneCollapsible.tsx | 12 ++++++++++++ 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/apps/web/e2e/product-specs-collapse.spec.ts b/apps/web/e2e/product-specs-collapse.spec.ts index 8129e840..4c664f2d 100644 --- a/apps/web/e2e/product-specs-collapse.spec.ts +++ b/apps/web/e2e/product-specs-collapse.spec.ts @@ -27,9 +27,18 @@ test('phone shows three spec groups and expands to all', async ({ page }) => { }); }).toPass(); await expect(groups.nth(total - 1)).toBeVisible(); - // Collapsed, the section is far shorter than the full ten-group stack. + // Collapsed, the section is shorter by the groups it hid. Empty groups are + // left out (DOS/Rate#1186), so the seeded phone has five short groups, not + // ten full ones, and a fixed ratio no longer says anything. const expandedHeight = (await section.boundingBox())?.height ?? 0; - expect(collapsedHeight).toBeLessThan(expandedHeight * 0.6); + let hiddenHeight = 0; + for (let i = 3; i < total; i++) { + hiddenHeight += (await groups.nth(i).boundingBox())?.height ?? 0; + } + expect(hiddenHeight).toBeGreaterThan(0); + expect(expandedHeight - collapsedHeight).toBeGreaterThanOrEqual( + hiddenHeight * 0.9 + ); }); test('desktop shows every spec group and no toggle', async ({ page }) => { diff --git a/apps/web/src/app/[locale]/smartphones/[slug]/page.tsx b/apps/web/src/app/[locale]/smartphones/[slug]/page.tsx index a68d6bd0..e9b67c67 100644 --- a/apps/web/src/app/[locale]/smartphones/[slug]/page.tsx +++ b/apps/web/src/app/[locale]/smartphones/[slug]/page.tsx @@ -1007,6 +1007,7 @@ export async function ProductDetailView({ showMoreLabel={t('specsShowAll')} showLessLabel={t('specsShowLess')} openOnHash="#specs-section" + collapsible={specGroups.length > 3} > {specGroups.map(({ group, rows }, i) => (
diff --git a/apps/web/src/components/product/PhoneCollapsible.tsx b/apps/web/src/components/product/PhoneCollapsible.tsx index fdf34e9e..9edba20f 100644 --- a/apps/web/src/components/product/PhoneCollapsible.tsx +++ b/apps/web/src/components/product/PhoneCollapsible.tsx @@ -12,6 +12,7 @@ export default function PhoneCollapsible({ showMoreLabel, showLessLabel, openOnHash, + collapsible = true, children, }: { className: string; @@ -19,6 +20,11 @@ export default function PhoneCollapsible({ showLessLabel: string; /** Expand when the page is opened or navigated to this hash, e.g. "#specs". */ openOnHash?: string; + /** + * False when there is nothing past the first three children: the list then + * shows whole, with no toggle (DOS/Rate#1186). + */ + collapsible?: boolean; children: ReactNode; }) { const [open, setOpen] = useState(false); @@ -46,6 +52,12 @@ export default function PhoneCollapsible({ } }; + if (!collapsible) { + return ( +
{children}
+ ); + } + return ( <>