Skip to content

Commit fc24d4b

Browse files
eschuthoclaude
andcommitted
fix: enforce minimum 2x scale on standard displays
Change IMAGE_DOWNLOAD_SCALE from `window.devicePixelRatio || 2` to `Math.max(window.devicePixelRatio || 1, 2)` so that standard displays (devicePixelRatio=1) also get 2x output. The previous expression evaluated to 1 on standard displays since 1 is truthy. Add tests covering the standard display (dpr=1→2) and zero (dpr=0→2) cases, and rename the prior fallback test accordingly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 00ab3a3 commit fc24d4b

File tree

2 files changed

+146
-1
lines changed

2 files changed

+146
-1
lines changed

superset-frontend/src/utils/downloadAsImage.test.ts

Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -683,3 +683,148 @@ test('clone path falls back to white background when theme is absent', async ()
683683

684684
document.body.removeChild(container);
685685
});
686+
687+
test('ag-grid path passes scale option to toJpeg', async () => {
688+
jest.useFakeTimers();
689+
const { container, agContainer, cleanup } = buildAgGridElement();
690+
attachMockApi(agContainer);
691+
692+
const handler = downloadAsImageOptimized('div', 'My Chart');
693+
const exportPromise = handler(syntheticEventFor(container));
694+
await jest.runAllTimersAsync();
695+
await exportPromise;
696+
697+
expect(mockToJpeg).toHaveBeenCalledWith(
698+
expect.any(HTMLElement),
699+
expect.objectContaining({ scale: expect.any(Number) }),
700+
);
701+
702+
cleanup();
703+
jest.useRealTimers();
704+
});
705+
706+
test('clone path passes scale option to toJpeg', async () => {
707+
const container = document.createElement('div');
708+
document.body.appendChild(container);
709+
710+
const handler = downloadAsImageOptimized('div', 'Bar Chart');
711+
await handler(syntheticEventFor(container));
712+
713+
expect(mockToJpeg).toHaveBeenCalledWith(
714+
expect.any(HTMLElement),
715+
expect.objectContaining({ scale: expect.any(Number) }),
716+
);
717+
718+
document.body.removeChild(container);
719+
});
720+
721+
test('scale uses window.devicePixelRatio when it is a positive number', async () => {
722+
let capturedScale: number | undefined;
723+
724+
await jest.isolateModulesAsync(async () => {
725+
Object.defineProperty(window, 'devicePixelRatio', {
726+
value: 3,
727+
configurable: true,
728+
});
729+
730+
jest.mock('dom-to-image-more', () => ({
731+
__esModule: true,
732+
default: {
733+
toJpeg: jest.fn((_el: HTMLElement, opts: { scale?: number }) => {
734+
capturedScale = opts.scale;
735+
return Promise.resolve('data:image/jpeg;base64,test');
736+
}),
737+
},
738+
}));
739+
jest.mock('src/components/MessageToasts/actions', () => ({
740+
addWarningToast: jest.fn(),
741+
}));
742+
jest.mock('@apache-superset/core/translation', () => ({
743+
t: (str: string) => str,
744+
}));
745+
746+
const { default: download } = await import('./downloadAsImage');
747+
const container = document.createElement('div');
748+
document.body.appendChild(container);
749+
await download('div', 'Chart')({ currentTarget: { closest: () => container } } as any);
750+
document.body.removeChild(container);
751+
});
752+
753+
expect(capturedScale).toBe(3);
754+
755+
Object.defineProperty(window, 'devicePixelRatio', {
756+
value: 1,
757+
configurable: true,
758+
});
759+
});
760+
761+
test('scale enforces a minimum of 2 on standard displays (devicePixelRatio=1)', async () => {
762+
let capturedScale: number | undefined;
763+
764+
await jest.isolateModulesAsync(async () => {
765+
Object.defineProperty(window, 'devicePixelRatio', {
766+
value: 1,
767+
configurable: true,
768+
});
769+
770+
jest.mock('dom-to-image-more', () => ({
771+
__esModule: true,
772+
default: {
773+
toJpeg: jest.fn((_el: HTMLElement, opts: { scale?: number }) => {
774+
capturedScale = opts.scale;
775+
return Promise.resolve('data:image/jpeg;base64,test');
776+
}),
777+
},
778+
}));
779+
jest.mock('src/components/MessageToasts/actions', () => ({
780+
addWarningToast: jest.fn(),
781+
}));
782+
jest.mock('@apache-superset/core/translation', () => ({
783+
t: (str: string) => str,
784+
}));
785+
786+
const { default: download } = await import('./downloadAsImage');
787+
const container = document.createElement('div');
788+
document.body.appendChild(container);
789+
await download('div', 'Chart')({ currentTarget: { closest: () => container } } as any);
790+
document.body.removeChild(container);
791+
});
792+
793+
// devicePixelRatio=1 is truthy, so || 2 would give 1 — Math.max enforces minimum 2
794+
expect(capturedScale).toBe(2);
795+
});
796+
797+
test('scale enforces a minimum of 2 when devicePixelRatio is 0', async () => {
798+
let capturedScale: number | undefined;
799+
800+
await jest.isolateModulesAsync(async () => {
801+
Object.defineProperty(window, 'devicePixelRatio', {
802+
value: 0,
803+
configurable: true,
804+
});
805+
806+
jest.mock('dom-to-image-more', () => ({
807+
__esModule: true,
808+
default: {
809+
toJpeg: jest.fn((_el: HTMLElement, opts: { scale?: number }) => {
810+
capturedScale = opts.scale;
811+
return Promise.resolve('data:image/jpeg;base64,test');
812+
}),
813+
},
814+
}));
815+
jest.mock('src/components/MessageToasts/actions', () => ({
816+
addWarningToast: jest.fn(),
817+
}));
818+
jest.mock('@apache-superset/core/translation', () => ({
819+
t: (str: string) => str,
820+
}));
821+
822+
const { default: download } = await import('./downloadAsImage');
823+
const container = document.createElement('div');
824+
document.body.appendChild(container);
825+
await download('div', 'Chart')({ currentTarget: { closest: () => container } } as any);
826+
document.body.removeChild(container);
827+
});
828+
829+
expect(capturedScale).toBe(2);
830+
});

superset-frontend/src/utils/downloadAsImage.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import { addWarningToast } from 'src/components/MessageToasts/actions';
2525
import type { AgGridContainerElement } from '@superset-ui/core/components';
2626

2727
const IMAGE_DOWNLOAD_QUALITY = 0.95;
28-
const IMAGE_DOWNLOAD_SCALE = window.devicePixelRatio || 2;
28+
const IMAGE_DOWNLOAD_SCALE = Math.max(window.devicePixelRatio || 1, 2);
2929
const TRANSPARENT_RGBA = 'transparent';
3030
const POLL_INTERVAL_MS = 100;
3131

0 commit comments

Comments
 (0)