fix(gallery): restore the download CTA under headerStyle "none"
The report asked whether this was intentional. It is collateral damage from the #386 swap, not intent. There are two header download affordances. GalleryView sets showDownloadAll={false} unconditionally -- "replaced by the new showHeaderDownload (#386)" -- and passes showHeaderDownload={allowDownloads}. GalleryLayout renders HeaderDownloadButton in the standard, minimal and hero branches, but the isNoHeader branch only ever had the now-dead showDownloadAll button. Net effect: zero download CTA on headerStyle 'none'. The comment claiming intent -- "Intentionally NOT shown in the no-header variant where the gallery is fully chromeless by design" -- is factually wrong about its own branch: isNoHeader renders the menu button, headerExtra (upload button, countdown timer) and logout. It is a functional-controls bar, not chromeless. The sentence predates the #386 swap, when showDownloadAll still gave that bar a download button. Renders HeaderDownloadButton in that branch in the same slot order as the other three; it is icon-only below sm, so it fits the compact bar. Removed the two now-false comments. Beta themes are unaffected: gallery-premium and gallery-story return from an earlier branch that never mounts GalleryLayout and get download-all via their own onDownloadEverything prop, so there is no double CTA. Refs testplan REPORT.md, headerStyle:none download-CTA warning.
This commit is contained in:
@@ -506,8 +506,7 @@ export const GalleryLayout: React.FC<GalleryLayoutProps> = ({
|
|||||||
)}
|
)}
|
||||||
{/* Accent Download CTA — also rendered in the minimal header
|
{/* Accent Download CTA — also rendered in the minimal header
|
||||||
so the action stays one click away regardless of header
|
so the action stays one click away regardless of header
|
||||||
style. Intentionally NOT shown in the no-header variant
|
style. */}
|
||||||
where the gallery is fully chromeless by design. */}
|
|
||||||
{showHeaderDownload && onHeaderDownload && (
|
{showHeaderDownload && onHeaderDownload && (
|
||||||
<HeaderDownloadButton
|
<HeaderDownloadButton
|
||||||
onClick={onHeaderDownload}
|
onClick={onHeaderDownload}
|
||||||
@@ -552,6 +551,17 @@ export const GalleryLayout: React.FC<GalleryLayoutProps> = ({
|
|||||||
<span className="hidden sm:inline">{t('gallery.downloadAll')}</span>
|
<span className="hidden sm:inline">{t('gallery.downloadAll')}</span>
|
||||||
</Button>
|
</Button>
|
||||||
)}
|
)}
|
||||||
|
{/* Accent Download CTA — 'none' suppresses the *title* header,
|
||||||
|
not the download affordance: this bar still renders the
|
||||||
|
menu, headerExtra and logout, so leaving the CTA out just
|
||||||
|
stranded guests with per-tile downloads only (QA P4-B.05). */}
|
||||||
|
{showHeaderDownload && onHeaderDownload && (
|
||||||
|
<HeaderDownloadButton
|
||||||
|
onClick={onHeaderDownload}
|
||||||
|
isDownloading={isDownloading}
|
||||||
|
label={t('gallery.download', 'Download')}
|
||||||
|
/>
|
||||||
|
)}
|
||||||
{showLogout && onLogout && (
|
{showLogout && onLogout && (
|
||||||
<Button
|
<Button
|
||||||
variant="outline"
|
variant="outline"
|
||||||
@@ -596,9 +606,7 @@ export const GalleryLayout: React.FC<GalleryLayoutProps> = ({
|
|||||||
)}
|
)}
|
||||||
|
|
||||||
{/* Accent Download CTA — also rendered above the hero so the
|
{/* Accent Download CTA — also rendered above the hero so the
|
||||||
primary download action is reachable without scrolling.
|
primary download action is reachable without scrolling. */}
|
||||||
Intentionally NOT shown in the no-header variant where
|
|
||||||
the gallery is fully chromeless by design. */}
|
|
||||||
{showHeaderDownload && onHeaderDownload && (
|
{showHeaderDownload && onHeaderDownload && (
|
||||||
<HeaderDownloadButton
|
<HeaderDownloadButton
|
||||||
onClick={onHeaderDownload}
|
onClick={onHeaderDownload}
|
||||||
|
|||||||
@@ -0,0 +1,97 @@
|
|||||||
|
/**
|
||||||
|
* The download CTA must survive every header style.
|
||||||
|
*
|
||||||
|
* `showDownloadAll` (the old primary "Download all" button) is hard-wired to
|
||||||
|
* false by GalleryView — the live entry point is `showHeaderDownload`, the
|
||||||
|
* accent CTA that opens the resolution picker. That CTA was rendered in the
|
||||||
|
* standard, minimal and hero headers but not in `headerStyle: 'none'`, whose
|
||||||
|
* comment claimed the variant was "fully chromeless by design" — it is not: it
|
||||||
|
* still renders the menu, headerExtra and logout. So `none` silently dropped
|
||||||
|
* the gallery's primary download affordance, leaving guests with per-tile
|
||||||
|
* downloads and the selection-mode bulk button only (QA P4-B.05).
|
||||||
|
*
|
||||||
|
* 'none' means "no title header", not "no downloads".
|
||||||
|
*/
|
||||||
|
import { render, screen } from '@testing-library/react';
|
||||||
|
import { describe, expect, it, vi } from 'vitest';
|
||||||
|
import { MemoryRouter } from 'react-router-dom';
|
||||||
|
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
|
||||||
|
import type { HeaderStyleType } from '../../../types/theme.types';
|
||||||
|
|
||||||
|
import { GalleryLayout } from '../GalleryLayout';
|
||||||
|
|
||||||
|
vi.mock('react-i18next', async () => {
|
||||||
|
const actual = await vi.importActual<typeof import('react-i18next')>('react-i18next');
|
||||||
|
return {
|
||||||
|
...actual,
|
||||||
|
useTranslation: () => ({
|
||||||
|
t: (key: string, second?: any) => (typeof second === 'string' ? second : key),
|
||||||
|
i18n: { language: 'en' },
|
||||||
|
}),
|
||||||
|
};
|
||||||
|
});
|
||||||
|
|
||||||
|
vi.mock('../../../contexts/ThemeContext', async () => {
|
||||||
|
const actual = await vi.importActual<typeof import('../../../contexts/ThemeContext')>(
|
||||||
|
'../../../contexts/ThemeContext'
|
||||||
|
);
|
||||||
|
return { ...actual, useTheme: () => ({ theme: {} }) };
|
||||||
|
});
|
||||||
|
|
||||||
|
vi.mock('../../../services/cms.service', () => ({
|
||||||
|
cmsService: { getPublicPage: vi.fn().mockRejectedValue(new Error('no cms')) },
|
||||||
|
}));
|
||||||
|
|
||||||
|
const renderLayout = (headerStyle: HeaderStyleType) => {
|
||||||
|
const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } });
|
||||||
|
return render(
|
||||||
|
<QueryClientProvider client={queryClient}>
|
||||||
|
<MemoryRouter>
|
||||||
|
<GalleryLayout
|
||||||
|
event={{ event_name: 'ZZTEST Wedding' }}
|
||||||
|
headerStyle={headerStyle}
|
||||||
|
showDownloadAll={false}
|
||||||
|
onDownloadAll={vi.fn()}
|
||||||
|
showHeaderDownload
|
||||||
|
onHeaderDownload={vi.fn()}
|
||||||
|
showLogout
|
||||||
|
onLogout={vi.fn()}
|
||||||
|
>
|
||||||
|
<div>photos</div>
|
||||||
|
</GalleryLayout>
|
||||||
|
</MemoryRouter>
|
||||||
|
</QueryClientProvider>
|
||||||
|
);
|
||||||
|
};
|
||||||
|
|
||||||
|
describe('GalleryLayout header download CTA', () => {
|
||||||
|
it.each<HeaderStyleType>(['standard', 'minimal', 'hero', 'none', 'banner'])(
|
||||||
|
'renders the download CTA with headerStyle "%s"',
|
||||||
|
(headerStyle) => {
|
||||||
|
const { unmount } = renderLayout(headerStyle);
|
||||||
|
expect(screen.getAllByRole('button', { name: 'Download' }).length).toBeGreaterThan(0);
|
||||||
|
unmount();
|
||||||
|
}
|
||||||
|
);
|
||||||
|
|
||||||
|
it('omits the CTA when the gallery does not allow downloads', () => {
|
||||||
|
const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } });
|
||||||
|
render(
|
||||||
|
<QueryClientProvider client={queryClient}>
|
||||||
|
<MemoryRouter>
|
||||||
|
<GalleryLayout
|
||||||
|
event={{ event_name: 'ZZTEST Wedding' }}
|
||||||
|
headerStyle="none"
|
||||||
|
showDownloadAll={false}
|
||||||
|
showHeaderDownload={false}
|
||||||
|
showLogout
|
||||||
|
onLogout={vi.fn()}
|
||||||
|
>
|
||||||
|
<div>photos</div>
|
||||||
|
</GalleryLayout>
|
||||||
|
</MemoryRouter>
|
||||||
|
</QueryClientProvider>
|
||||||
|
);
|
||||||
|
expect(screen.queryByRole('button', { name: 'Download' })).not.toBeInTheDocument();
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user