Compare commits

...
Author SHA1 Message Date
bermudalamb f84050c689 docs: adopt Conventional Branch naming alongside Conventional Commits
SonarQube Analysis / sonarqube (pull_request) Successful in 2m57s
Tests / backend-unit (pull_request) Successful in 38s
Tests / frontend-e2e (pull_request) Failing after 2m51s
Branches follow Conventional Branch — `<type>/<description>` with types feature, bugfix, hotfix, release, chore — carrying the issue number so the work is identifiable from `git branch`: `feature/48-my-account-modal`. That is the same `feature/` prefix this repo has always used, so no existing branch was named wrongly. Commits stay Conventional Commits with the issue number appended to the subject and a `Closes #N` line in the body.

Spells out which half does what, because it is easy to assume the branch name links the work: Gitea builds the reference from a `#N` in a commit message or PR and never from the branch name. The name is for humans; the reference is what ties the work to the issue.

Also records the backdrop-location arrangement in `AppRoutes`, since it is the pattern the sibling navigational dead-end issues should copy rather than each inventing their own, and folds the unwrapped-commit-body rule in with the rest of the commit conventions.
2026-08-18 15:57:14 -05:00
bermudalamb 534513c760 feat(account): open My Account as a modal over the page behind it (#48)
/account had no site header and no links of any kind, so once a customer opened it the only way out was the browser's back button or editing the URL.

It is now a modal rendered over whatever the customer was looking at, while staying a real route. Opening it from the header pushes /account and names the current page as the backdrop, so closing returns there with filters and scroll position intact, and the browser's Back button does the same thing as the close control. Entering /account directly — a bookmark, the link in a verification email, or the redirect after registering — has no page behind it, so it falls back to rendering the storefront as the backdrop. Closing therefore always lands somewhere real rather than on nothing.

Keeping it a route rather than view state means the URL still works: it can be bookmarked, shared, and refreshed with the account view still open, which is what the existing header link and the four post-authentication redirects already depend on.

Also scopes the account switch locator in the favorites spec to the modal. The storefront now renders behind the account view and has a theme switch of its own, so an unscoped switch locator was only picking the right control by DOM accident.

Verified with 68 end-to-end tests, 5 of them new, all passing, and type checking clean. No backend changes.

Closes #48
2026-08-18 15:47:00 -05:00
6 changed files with 191 additions and 27 deletions
+7 -2
View File
@@ -213,8 +213,12 @@ sudo docker exec -it redefined-designs-db-syn psql -U redefined -d redefined -c
## Conventions ## Conventions
- **Every commit message follows [Conventional Commits](https://www.conventionalcommits.org/)**: `feat:`, `fix:`, `chore:`, `docs:`, `test:`, `ci:`, `refactor:`, etc. - **Never commit directly to `main`.** Always branch, open a PR, merge; the branch auto-deletes (repo setting is on).
- **Never commit directly to `main`.** Always branch: `feature/<short-description>` or `fix/<short-description>`. Open a PR, merge, branch auto-deletes (repo setting is on). - **Branches follow [Conventional Branch](https://conventional-branch.github.io/), with the issue number carried for Gitea:** `<type>/<issue-number>-<short-slug>`, e.g. `feature/48-my-account-modal`, `bugfix/57-cart-total-wrong`. Types are `feature`, `bugfix`, `hotfix`, `release`, `chore` — the same `feature/` prefix this repo has always used, so nothing in the existing history is wrong. Drop the number when there is no issue behind the work (`chore/tidy-dead-routes`). The type should agree with the Conventional Commit type of the work it carries.
- **Commits follow [Conventional Commits](https://www.conventionalcommits.org/)** — `feat:`, `fix:`, `chore:`, `docs:`, `test:`, `ci:`, `refactor:` — with the issue number appended to the subject: `feat(account): open My Account as a modal (#48)`.
- **Put `Closes #48` in the commit body**, on its own line, for the commit that completes the issue (`Refs #48` when it only contributes). This is what actually closes the issue on merge, independently of whether the PR description repeats it.
- **Be clear about which part does the linking.** Gitea creates the reference from a `#48` appearing in a *commit message or PR* — never from the branch name. The branch name is for humans reading `git branch`; the reference is what ties the work to the issue. Both are wanted, but only one of them links.
- **Commit bodies are unwrapped paragraphs** — no hard line breaks inside a paragraph.
- Local dev/editing happens in **VS Code**, pushed via **PowerShell** `git` — the NAS-side `gitc` workflow is *only* for pulling already-merged code down to deploy, never for authoring changes. - Local dev/editing happens in **VS Code**, pushed via **PowerShell** `git` — the NAS-side `gitc` workflow is *only* for pulling already-merged code down to deploy, never for authoring changes.
- **Thom does the pushing.** Commit locally and stop; don't `git push` on his behalf. - **Thom does the pushing.** Commit locally and stop; don't `git push` on his behalf.
- **When work comes from a Gitea issue, post every clarifying question and its answer back to that issue as a comment** — including the options considered and why the rejected ones were rejected. The issue is the durable record; decisions made in a chat session are invisible to anyone reading it later. Post each round as the answers come in rather than batching everything to the end. - **When work comes from a Gitea issue, post every clarifying question and its answer back to that issue as a comment** — including the options considered and why the rejected ones were rejected. The issue is the durable record; decisions made in a chat session are invisible to anyone reading it later. Post each round as the answers come in rather than batching everything to the end.
@@ -259,6 +263,7 @@ Integration tests truncate between cases (`resetDb()` in `tests/integration/setu
- Change admin-configurable settings → `admin_settings` table + `backend/src/routes/adminSettings.ts` + `frontend/src/admin/Settings.tsx` - Change admin-configurable settings → `admin_settings` table + `backend/src/routes/adminSettings.ts` + `frontend/src/admin/Settings.tsx`
- Add a new async route → wrap the handler in `asyncRoute()` from `backend/src/asyncRoute.ts`, or a failure will hang the request instead of returning 500 - Add a new async route → wrap the handler in `asyncRoute()` from `backend/src/asyncRoute.ts`, or a failure will hang the request instead of returning 500
- Change what an item row returns → `backend/src/itemSelect.ts` (one place, used by both the public and admin routes) - Change what an item row returns → `backend/src/itemSelect.ts` (one place, used by both the public and admin routes)
- Change how a customer route is framed (modal vs page) → `frontend/src/main.tsx`. `AppRoutes` renders the route table against a *backdrop* location rather than the real one: `/account` is a modal over the page named in `location.state.background`, falling back to the storefront when there is none (a bookmark, an email link, a post-registration redirect). This is the pattern to copy for the sibling dead-end issues (#49, #50) — it keeps the URL real and linkable while making sure closing always lands somewhere. The link that opens it must pass `state={{ background: location }}`, or closing goes to the fallback instead of where the customer was.
- Change storefront filtering → `backend/src/itemFilters.ts` (parsing + SQL), `backend/src/routes/filters.ts` (`/api/filters`, the drawer's single fetch), `frontend/src/filters.ts` (state, URL round-trip, tree building), `frontend/src/components/FilterDrawer.tsx` - Change storefront filtering → `backend/src/itemFilters.ts` (parsing + SQL), `backend/src/routes/filters.ts` (`/api/filters`, the drawer's single fetch), `frontend/src/filters.ts` (state, URL round-trip, tree building), `frontend/src/components/FilterDrawer.tsx`
- Change category/tag management → `backend/src/routes/adminCategories.ts`, `backend/src/routes/adminTags.ts`, `frontend/src/admin/Categories.tsx`, `frontend/src/admin/Tags.tsx` - Change category/tag management → `backend/src/routes/adminCategories.ts`, `backend/src/routes/adminTags.ts`, `frontend/src/admin/Categories.tsx`, `frontend/src/admin/Tags.tsx`
- Change tag colours → `TAG_COLORS` + `tagColorFor()` in `backend/src/utils.ts`; the list is **duplicated** in `frontend/src/admin/Tags.tsx` for the override picker, and the server rejects anything outside it, so the two must be changed together - Change tag colours → `TAG_COLORS` + `tagColorFor()` in `backend/src/utils.ts`; the list is **duplicated** in `frontend/src/admin/Tags.tsx` for the override picker, and the server rejects anything outside it, so the two must be changed together
+8 -2
View File
@@ -1,7 +1,7 @@
import { useEffect, useState, useCallback, useMemo } from 'react'; import { useEffect, useState, useCallback, useMemo } from 'react';
import { Layout, Typography, Switch, Row, Col, Spin, Button, theme, Badge, Empty, Alert } from 'antd'; import { Layout, Typography, Switch, Row, Col, Spin, Button, theme, Badge, Empty, Alert } from 'antd';
import { ShoppingCartOutlined, FilterOutlined } from '@ant-design/icons'; import { ShoppingCartOutlined, FilterOutlined } from '@ant-design/icons';
import { Link, useSearchParams } from 'react-router-dom'; import { Link, useLocation, useSearchParams } from 'react-router-dom';
import { Item, FilterOptions, fetchItems, fetchFilterOptions } from './api'; import { Item, FilterOptions, fetchItems, fetchFilterOptions } from './api';
import ItemCard from './components/ItemCard'; import ItemCard from './components/ItemCard';
import FilterDrawer from './components/FilterDrawer'; import FilterDrawer from './components/FilterDrawer';
@@ -31,6 +31,7 @@ export default function App() {
const [options, setOptions] = useState<FilterOptions | null>(null); const [options, setOptions] = useState<FilterOptions | null>(null);
const [drawerOpen, setDrawerOpen] = useState(false); const [drawerOpen, setDrawerOpen] = useState(false);
const [searchParams, setSearchParams] = useSearchParams(); const [searchParams, setSearchParams] = useSearchParams();
const location = useLocation();
const { mode, toggle } = useThemeMode(); const { mode, toggle } = useThemeMode();
const { customer } = useCustomerAuth(); const { customer } = useCustomerAuth();
const { items: cartItems } = useCart(); const { items: cartItems } = useCart();
@@ -105,8 +106,13 @@ export default function App() {
<Button icon={<ShoppingCartOutlined />} /> <Button icon={<ShoppingCartOutlined />} />
</Badge> </Badge>
</Link> </Link>
{/* The account link names the page to render behind the modal, so
closing it comes back here — filters and all — rather than to a
default. */}
{customer ? ( {customer ? (
<Link to="/account"><Button>My Account</Button></Link> <Link to="/account" state={{ background: location }}>
<Button>My Account</Button>
</Link>
) : ( ) : (
<> <>
<Link to="/login"><Button>Log in</Button></Link> <Link to="/login"><Button>Log in</Button></Link>
+22 -7
View File
@@ -1,5 +1,5 @@
import { useEffect, useState } from 'react'; import { useEffect, useState } from 'react';
import { Card, Typography, Switch, Button, Table, Modal, message, Space, Divider } from 'antd'; import { Typography, Switch, Button, Table, Modal, message, Space, Divider } from 'antd';
import { useNavigate } from 'react-router-dom'; import { useNavigate } from 'react-router-dom';
import { fetchMyOrders, OrderHistoryItem, updateConsent, exportMyData, deleteMyAccount } from './customerApi'; import { fetchMyOrders, OrderHistoryItem, updateConsent, exportMyData, deleteMyAccount } from './customerApi';
import { setFavoriteAlerts } from './favoritesApi'; import { setFavoriteAlerts } from './favoritesApi';
@@ -7,7 +7,13 @@ import { useCustomerAuth } from './CustomerAuthContext';
const { Title, Text } = Typography; const { Title, Text } = Typography;
export default function Account() { interface Props {
// Supplied by the route, which decides where closing lands: back to the page
// the customer came from, or to the storefront when they arrived directly.
onClose: () => void;
}
export default function Account({ onClose }: Props) {
const { customer, loading, refresh, logout } = useCustomerAuth(); const { customer, loading, refresh, logout } = useCustomerAuth();
const [orders, setOrders] = useState<OrderHistoryItem[]>([]); const [orders, setOrders] = useState<OrderHistoryItem[]>([]);
const navigate = useNavigate(); const navigate = useNavigate();
@@ -66,9 +72,18 @@ export default function Account() {
} }
return ( return (
<div style={{ maxWidth: 700, margin: '48px auto', padding: '0 16px' }}> <Modal
<Card> open
<Title level={3}>My Account</Title> // The account view is a place a customer can be sent by an email link or
// a bookmark, so it is titled and closable rather than relying on the
// page behind it to say where they are.
title="My Account"
onCancel={onClose}
footer={null}
width={700}
destroyOnHidden
>
<div>
<Text>{customer.email}</Text> <Text>{customer.email}</Text>
{!customer.email_verified && ( {!customer.email_verified && (
<div style={{ marginTop: 8 }}> <div style={{ marginTop: 8 }}>
@@ -112,7 +127,7 @@ export default function Account() {
<Button onClick={handleLogout}>Log out</Button> <Button onClick={handleLogout}>Log out</Button>
<Button danger onClick={handleDelete}>Delete my account</Button> <Button danger onClick={handleDelete}>Delete my account</Button>
</Space> </Space>
</Card> </div>
</div> </Modal>
); );
} }
+50 -13
View File
@@ -1,6 +1,7 @@
import React, { useEffect, useState } from 'react'; import React, { useEffect, useState } from 'react';
import ReactDOM from 'react-dom/client'; import ReactDOM from 'react-dom/client';
import { BrowserRouter, Routes, Route } from 'react-router-dom'; import { BrowserRouter, Routes, Route, useLocation, useNavigate } from 'react-router-dom';
import type { Location } from 'react-router-dom';
import { ConfigProvider, theme as antdTheme } from 'antd'; import { ConfigProvider, theme as antdTheme } from 'antd';
import 'antd/dist/reset.css'; import 'antd/dist/reset.css';
import App from './App'; import App from './App';
@@ -26,6 +27,11 @@ const DARK_ACCENT = '#f0f0f0';
const REDUCED_MOTION_QUERY = '(prefers-reduced-motion: reduce)'; const REDUCED_MOTION_QUERY = '(prefers-reduced-motion: reduce)';
// What /account renders over when it was entered directly — a bookmark, a link
// in an email, or a redirect after signing in. There is no page behind in that
// case, and a modal floating on nothing has nowhere to close back to.
const STOREFRONT_BACKDROP: Partial<Location> = { pathname: '/', search: '', hash: '' };
// Respects the OS-level "reduce motion" accessibility setting by turning off // Respects the OS-level "reduce motion" accessibility setting by turning off
// antd's transitions. Beyond the accessibility win, animated popups are a // antd's transitions. Beyond the accessibility win, animated popups are a
// standing source of flake in end-to-end tests, which drive the app with this // standing source of flake in end-to-end tests, which drive the app with this
@@ -45,6 +51,48 @@ function usePrefersReducedMotion(): boolean {
return prefers; return prefers;
} }
// /account is a route that renders as a modal over whatever the customer was
// looking at, rather than a page of its own. It stays a real, linkable URL —
// bookmarkable, refreshable, and closed by the browser's Back button — while
// never being a place with no way out of it.
function AppRoutes() {
const location = useLocation();
const navigate = useNavigate();
const state = location.state as { background?: Location } | null;
const isAccount = location.pathname === '/account';
// In-app navigation names the page to render behind. Anything else — a
// bookmark, an email link, the redirect after registering — falls back to the
// storefront, so closing always lands somewhere real.
const background = state?.background;
const backdrop = isAccount ? background ?? { ...location, ...STOREFRONT_BACKDROP } : location;
function closeAccount() {
// Back, when there is somewhere to go back to, so closing the modal and
// pressing Back do the same thing and neither leaves a dead entry behind.
if (background) navigate(-1);
else navigate('/', { replace: true });
}
return (
<>
<Routes location={backdrop as Location}>
<Route path="/" element={<App />} />
<Route path="/admin" element={<Admin />} />
<Route path="/login" element={<Login />} />
<Route path="/register" element={<Register />} />
<Route path="/cart" element={<Cart />} />
<Route path="/privacy" element={<PrivacyPolicy />} />
<Route path="/verify-email" element={<VerifyEmail />} />
<Route path="/forgot-password" element={<ForgotPassword />} />
<Route path="/reset-password" element={<ResetPassword />} />
</Routes>
{/* Rendered outside the Routes above, which are showing the backdrop. */}
{isAccount && <Account onClose={closeAccount} />}
</>
);
}
function Root() { function Root() {
const { mode } = useThemeMode(); const { mode } = useThemeMode();
const prefersReducedMotion = usePrefersReducedMotion(); const prefersReducedMotion = usePrefersReducedMotion();
@@ -66,18 +114,7 @@ function Root() {
}} }}
> >
<BrowserRouter> <BrowserRouter>
<Routes> <AppRoutes />
<Route path="/" element={<App />} />
<Route path="/admin" element={<Admin />} />
<Route path="/login" element={<Login />} />
<Route path="/register" element={<Register />} />
<Route path="/account" element={<Account />} />
<Route path="/cart" element={<Cart />} />
<Route path="/privacy" element={<PrivacyPolicy />} />
<Route path="/verify-email" element={<VerifyEmail />} />
<Route path="/forgot-password" element={<ForgotPassword />} />
<Route path="/reset-password" element={<ResetPassword />} />
</Routes>
</BrowserRouter> </BrowserRouter>
</ConfigProvider> </ConfigProvider>
); );
+98
View File
@@ -0,0 +1,98 @@
import { test, expect, Page } from '@playwright/test';
const PASSWORD = 'supersecret123';
const uniqueEmail = () => `account-${Date.now().toString(36)}${Math.random().toString(36).slice(2, 7)}@example.com`;
// Registering lands on /account, which is now the modal over the storefront.
// Returns the address so a test can assert the right account is shown.
async function registerAndCloseAccount(page: Page): Promise<string> {
const email = uniqueEmail();
await page.goto('/register');
await page.getByRole('textbox', { name: 'Email' }).fill(email);
await page.getByLabel('Password').fill(PASSWORD);
await page.getByRole('button', { name: 'Create account' }).click();
await expect(page).toHaveURL(/\/account/);
await closeAccount(page);
return email;
}
const accountModal = (page: Page) => page.getByRole('dialog', { name: 'My Account' });
async function closeAccount(page: Page) {
await accountModal(page).getByRole('button', { name: 'Close' }).click();
await expect(accountModal(page)).toBeHidden();
}
test.describe('My Account opens as a modal', () => {
test('opens over the storefront and closes back to it, filters and all', async ({ page }) => {
await registerAndCloseAccount(page);
// A filtered view, to prove closing restores where the customer actually
// was rather than a bare storefront.
await page.goto('/?max_price=50000');
await expect(page.getByRole('button', { name: 'My Account' })).toBeVisible();
await page.getByRole('button', { name: 'My Account' }).click();
await expect(accountModal(page)).toBeVisible();
// Still a real, linkable URL rather than hidden view state.
await expect(page).toHaveURL(/\/account/);
await closeAccount(page);
await expect(page).toHaveURL(/max_price=50000/);
});
test('the browser back button closes it, the same as the close control', async ({ page }) => {
await registerAndCloseAccount(page);
await page.goto('/?max_price=50000');
await page.getByRole('button', { name: 'My Account' }).click();
await expect(accountModal(page)).toBeVisible();
await page.goBack();
await expect(accountModal(page)).toBeHidden();
await expect(page).toHaveURL(/max_price=50000/);
});
test('a direct visit renders the storefront behind it, so closing lands somewhere real', async ({ page }) => {
await registerAndCloseAccount(page);
// A bookmark, or the link in a verification email. There is no page behind
// in this case, which is what used to make /account a dead end.
await page.goto('/account');
await expect(accountModal(page)).toBeVisible();
await expect(page.getByRole('heading', { name: 'Redefined Designs' })).toBeVisible();
await closeAccount(page);
await expect(page).toHaveURL(/\/$/);
await expect(page.getByRole('button', { name: 'My Account' })).toBeVisible();
});
test('survives a reload, since it is a route rather than view state', async ({ page }) => {
const email = await registerAndCloseAccount(page);
await page.goto('/?max_price=50000');
await page.getByRole('button', { name: 'My Account' }).click();
await expect(accountModal(page)).toBeVisible();
await page.reload();
await expect(accountModal(page)).toBeVisible();
await expect(accountModal(page)).toContainText(email);
});
test('shows the signed-in account and its settings', async ({ page }) => {
const email = await registerAndCloseAccount(page);
await page.goto('/account');
const modal = accountModal(page);
await expect(modal).toContainText(email);
await expect(modal).toContainText('Order History');
// Scoped to the modal: the storefront behind it has a theme switch of its
// own, so an unscoped switch locator would be ambiguous.
await expect(modal.getByRole('switch')).toHaveCount(2);
});
});
+6 -3
View File
@@ -130,10 +130,13 @@ test.describe('Favoriting items', () => {
await page.request.put('/api/customers/me/favorite-alerts', { data: { enabled: true } }); await page.request.put('/api/customers/me/favorite-alerts', { data: { enabled: true } });
await page.goto('/account'); await page.goto('/account');
const toggle = page.getByText('Email me when an item I favorited is sold'); // Scoped to the account modal. The storefront renders behind it and has a
await expect(toggle).toBeVisible(); // theme switch of its own, so an unscoped switch locator only picks the
// right control by DOM accident.
const account = page.getByRole('dialog', { name: 'My Account' });
await expect(account.getByText('Email me when an item I favorited is sold')).toBeVisible();
await page.getByRole('switch').last().click(); await account.getByRole('switch').last().click();
await expect(page.getByText('Turned off')).toBeVisible(); await expect(page.getByText('Turned off')).toBeVisible();
const me = await (await page.request.get('/api/customers/me')).json(); const me = await (await page.request.get('/api/customers/me')).json();