Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions apps/frontend/eslint.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,14 @@ export default defineConfigWithVueTs(
element: 'dialog',
message: 'Add a shared dialog to shared/ui rather than a local one — see frontend ADR 0003.',
},
// Structure, closed once the two screens that drew their own had
// somewhere to go (#85). A table is not an affordance, but hand-writing
// one is how a surface ends up deciding what a row looks like — which
// is the boundary frontend ADR 0004 draws.
...['table', 'thead', 'tbody', 'tr', 'th', 'td'].map((element) => ({
element,
message: 'Use Table from shared/ui with typed cells — see frontend ADR 0004.',
})),
],

// A ban reading tag names alone waves through the hand-rolled equivalent:
Expand Down
35 changes: 35 additions & 0 deletions apps/frontend/scripts/check-text-styles.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,27 @@ function* files(directory) {
}
}

/*
* The `font` shorthand resets `font-family`, so a mono declaration placed
* before it is silently undone. That is not hypothetical: the catalogue's
* merchant reference shipped without its monospace face for exactly this
* reason, and neither the type check, the lint, nor a look at the screenshot
* caught it — a pixel comparison did.
*/
function familyUndoneByShorthand(text) {
const found = []
for (const [, selector, block] of text.matchAll(/([^{}]*)\{([^{}]*)\}/g)) {
const family = block.indexOf('font-family:')
const shorthand = block.indexOf('font: var(--style-')
if (family !== -1 && shorthand !== -1 && shorthand > family) {
found.push(selector.trim().split('\n').pop().trim())
}
}
return found
}

const violations = []
const undone = []

for (const path of files(SOURCE)) {
if (ALLOWED.includes(path)) {
Expand All @@ -68,6 +88,21 @@ for (const path of files(SOURCE)) {
}
}
})

for (const selector of familyUndoneByShorthand(readFileSync(path, 'utf8'))) {
undone.push(`${relative(ROOT, path)} ${selector}`)
}
}

if (undone.length > 0) {
console.error(
`${undone.length} rule(s) where \`font:\` undoes the \`font-family\` above it.\n` +
'The shorthand resets the family: declare it after, not before.\n',
)
for (const one of undone) {
console.error(` ${one}`)
}
process.exit(1)
}

if (violations.length > 0) {
Expand Down
5 changes: 3 additions & 2 deletions apps/frontend/src/i18n/locales/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@
"shop": "The shop",
"shopAbout": "{country} · {currency} · {timezone}. Everyone who administers this shop sees the same.",
"rates": {
"title": "VAT rates",
"name": "Name",
"rate": "Rate",
"add": "Add",
Expand All @@ -141,8 +142,8 @@
"name": "Name",
"legalIdentity": "Legal name and registration number",
"country": "Country of the shop",
"countryDecides": "Sets which VAT rates you can charge.",
"currency": "Currency",
"countryDecides": "Sets which VAT rates you can charge.",
"currency": "Currency",
"currencyFinal": "Final once the first order is placed.",
"timezone": "Time zone",
"contentLanguage": "Product page language",
Expand Down
5 changes: 3 additions & 2 deletions apps/frontend/src/i18n/locales/fr.json
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@
"shop": "La boutique",
"shopAbout": "{country} · {currency} · {timezone}. Tous ceux qui administrent cette boutique voient la même chose.",
"rates": {
"title": "Taux de TVA",
"name": "Nom",
"rate": "Taux",
"add": "Ajouter",
Expand All @@ -141,8 +142,8 @@
"name": "Nom",
"legalIdentity": "Raison sociale et immatriculation",
"country": "Pays de la boutique",
"countryDecides": "Détermine les taux de TVA que vous pouvez appliquer.",
"currency": "Devise",
"countryDecides": "Détermine les taux de TVA que vous pouvez appliquer.",
"currency": "Devise",
"currencyFinal": "Définitive dès la première commande.",
"timezone": "Fuseau horaire",
"contentLanguage": "Langue des fiches produits",
Expand Down
24 changes: 24 additions & 0 deletions apps/frontend/src/shared/ui/Table.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,30 @@ describe('Table', () => {
expect(onPress).not.toHaveBeenCalled()
})

it('renders no header row when no column names anything', () => {
stubMatchMedia(false)
// The rates screen is such a table: a name, a figure, a mark, an action —
// each reads for itself, and a row of empty headers above them is chrome
// for nothing.
const rendered = table({
columns: [{ key: 'name', header: '' }, { key: 'act', header: '' }],
rows: [{ key: 1, cells: { name: { kind: 'strong', value: 'Standard' } } }],
})

expect(rendered.find('thead').exists()).toBe(false)
expect(rendered.find('tbody').exists()).toBe(true)
})

it('renders the header row as soon as one column names something', () => {
stubMatchMedia(false)
const rendered = table({
columns: [{ key: 'name', header: 'Taux' }, { key: 'act', header: '' }],
rows: [{ key: 1, cells: { name: { kind: 'strong', value: 'Standard' } } }],
})

expect(rendered.find('thead').exists()).toBe(true)
})

it('renders a cell a row never filed, rather than failing on it', () => {
stubMatchMedia(false)
// A column whose key no row carries: the table shows the absence instead
Expand Down
11 changes: 10 additions & 1 deletion apps/frontend/src/shared/ui/Table.vue
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,15 @@ function cellOf(row: Row, column: Column): Cell {
}

const hasRows = computed(() => props.rows.length > 0)

/**
* No header row when no column names anything.
*
* The rates screen is such a table: four columns, none of them a rubric — a
* name, a figure, a mark and an action read for themselves. A row of empty
* headers above them would be chrome for nothing.
*/
const hasHeaders = computed(() => props.columns.some((column) => column.header !== ''))
</script>

<template>
Expand All @@ -57,7 +66,7 @@ const hasRows = computed(() => props.rows.length > 0)
v-else-if="!narrow"
:aria-label="label"
>
<thead>
<thead v-if="hasHeaders">
<tr>
<th
v-for="column in columns"
Expand Down
16 changes: 14 additions & 2 deletions apps/frontend/src/surfaces/admin/CatalogueView.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,13 @@ describe('CatalogueView', () => {
page: page([product({ merchantReference: undefined })]),
})

expect((await catalogue()).find('td.reference').text()).toBe('—')
const rendered = await catalogue()

// What a merchant reads, not where it is written: a dash stands in for the
// reference, and no blank cell and no "undefined" reaches the screen.
expect(rendered.text()).toContain('—')
expect(rendered.text()).not.toContain('undefined')
expect(rendered.text()).not.toContain('null')
})

it('names the state in words, not in colour alone', async () => {
Expand All @@ -88,7 +94,13 @@ describe('CatalogueView', () => {
page: page([product({ state: 'draft' })]),
})

expect((await catalogue()).find('.state').text()).toBe('Draft')
const rendered = await catalogue()

// WCAG 1.4.1: the state is readable without seeing a colour. Asserted on
// the words the screen shows, so the guarantee survives the markup that
// happens to carry them — this test broke once on exactly that.
expect(rendered.text()).toContain('Draft')
expect(rendered.text()).not.toContain('draft')
})

/**
Expand Down
145 changes: 5 additions & 140 deletions apps/frontend/src/surfaces/admin/CatalogueView.vue
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,10 @@ import { useI18n } from 'vue-i18n'

import Button from '@/shared/ui/Button.vue'
import PageTitle from '@/shared/ui/PageTitle.vue'
import ProductTable from '@/surfaces/admin/ProductTable.vue'
import { listProducts, type ProductPage } from '@/shared/api/catalogue'
import { useNarrowScreen } from '@/composables/useNarrowScreen'
import { formatAmount, formatRate } from '@/shared/money'
import { useShopStore } from '@/stores/shop'

const { t, locale } = useI18n()
const shop = useShopStore()
const { narrow } = useNarrowScreen()
const { t } = useI18n()

const page = ref<ProductPage | undefined>(undefined)
const unreachable = ref(false)
Expand All @@ -31,10 +27,6 @@ const range = computed(() => {
}
})

function price(minor: number): string {
// One currency per shop, so a missing one means the shop was never asked.
return shop.currency ? formatAmount(minor, shop.currency, locale.value) : String(minor)
}

async function load(): Promise<void> {
const outcome = await listProducts()
Expand Down Expand Up @@ -72,93 +64,10 @@ onMounted(load)

<!-- One screen: the table when there are products, a sentence where the
table would be when there are none. No second design to maintain
(docs/design/catalog.md § 7). -->
<p
v-else-if="products.length === 0"
class="empty"
>
{{ t('catalogue.empty') }}
</p>

(docs/design/catalog.md § 7) — the sentence is the table's own empty
state, so there is nothing here deciding between the two. -->
<template v-else>
<table v-if="!narrow">
<thead>
<tr>
<th class="text-label">
{{ t('catalogue.column.product') }}
</th>
<th class="text-label">
{{ t('catalogue.column.reference') }}
</th>
<th class="number text-label">
{{ t('catalogue.column.price') }}
</th>
<th
v-if="shop.vatEnabled"
class="number text-label"
>
{{ t('catalogue.column.vat') }}
</th>
<th class="text-label">
{{ t('catalogue.column.state') }}
</th>
</tr>
</thead>
<tbody>
<tr
v-for="product in products"
:key="product.id"
>
<td class="name">
{{ product.title }}
</td>
<td class="reference">
{{ product.merchantReference ?? '—' }}
</td>
<td class="number">
{{ price(product.price) }}
</td>
<!-- Which rate, not how much tax: the amount follows from the
price already in the row, the rate is what a merchant scans
for when a law changes. -->
<td
v-if="shop.vatEnabled"
class="number"
>
{{ product.vatBasisPoints == null ? '—' : formatRate(product.vatBasisPoints, locale) }}
</td>
<td>
<span :class="['state', product.state]">{{ t(`catalogue.state.${product.state}`) }}</span>
</td>
</tr>
</tbody>
</table>

<!-- Four columns in 412 px leave each about 25 characters, which is a
table nobody can read: the row becomes a stacked card. -->
<ul
v-else
class="cards"
>
<li
v-for="product in products"
:key="product.id"
>
<span class="name">{{ product.title }}</span>
<span class="meta">
<span class="number">{{ price(product.price) }}</span>
<span
v-if="shop.vatEnabled && product.vatBasisPoints != null"
class="vat"
>{{ t('catalogue.column.vat') }} {{ formatRate(product.vatBasisPoints, locale) }}</span>
<span :class="['state', product.state]">{{ t(`catalogue.state.${product.state}`) }}</span>
<span
v-if="product.merchantReference"
class="reference"
>{{ product.merchantReference }}</span>
</span>
</li>
</ul>
<ProductTable :products="products" />

<p
v-if="range"
Expand Down Expand Up @@ -225,17 +134,6 @@ td.name {
font: var(--style-body-strong);
}

.reference {
color: var(--colour-text-muted);
font-family: var(--font-mono);
font: var(--style-caption);
}

.number {
font-variant-numeric: tabular-nums;
text-align: right;
white-space: nowrap;
}

.state {
display: inline-flex;
Expand All @@ -254,42 +152,9 @@ td.name {
color: var(--colour-text-muted);
}

.cards {
display: flex;
flex-direction: column;
gap: var(--space-2);
margin: 0;
padding: 0;
list-style: none;
}

.cards li {
display: flex;
flex-direction: column;
gap: var(--space-1);
padding: var(--space-2) var(--space-3);
border: 1px solid var(--colour-border);
border-radius: var(--radius-2);
background: var(--colour-surface-raised);
}

.cards .name {
font: var(--style-body-strong);
}

.cards .meta {
display: flex;
gap: var(--space-3);
align-items: center;
flex-wrap: wrap;
font: var(--style-caption);
}

.cards .number {
text-align: left;
}

.cards .vat {
color: var(--colour-text-muted);
}
</style>
Loading