Skip to content

Commit 4cbcb91

Browse files
silverwindwxiaoguang
silverwind
and
wxiaoguang
authored
Fix SVG size calulation, only use style attribute (#36133)
Fixes: #35863 The old code had a conflict between using HTML attributes vs. style properties where the style was overriding the previously set HTML attributes: ```html <img width="300" height="277.02439470988946" style="width: 275px; height: 0px;"> ``` I made it so in all cases only `style` properties are used and the previous width/height values are now set via `style`. Also I did a number of much-needed typescript improvements to the file. --------- Co-authored-by: wxiaoguang <[email protected]>
1 parent bfbc38f commit 4cbcb91

File tree

4 files changed

+115
-88
lines changed

4 files changed

+115
-88
lines changed

‎web_src/css/base.css‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@
3939

4040
--gap-inline: 0.25rem; /* gap for inline texts and elements, for example: the spaces for sentence with labels, button text, etc */
4141
--gap-block: 0.5rem; /* gap for element blocks, for example: spaces between buttons, menu image & title, header icon & title etc */
42+
43+
--background-view-image: url("data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAoAAAAKCAYAAACNMs+9AAAAG0lEQVQYlWN4+vTpf3SMDTAMBYXYBLFpHgoKAeiOf0SGE9kbAAAAAElFTkSuQmCC") right bottom var(--color-primary-light-7);
4244
}
4345

4446
@media (min-width: 768px) and (max-width: 1200px) {

‎web_src/css/features/imagediff.css‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313

1414
.image-diff-container img {
1515
border: 1px solid var(--color-primary-light-7);
16-
background: url("data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAoAAAAKCAYAAACNMs+9AAAAG0lEQVQYlWN4+vTpf3SMDTAMBYXYBLFpHgoKAeiOf0SGE9kbAAAAAElFTkSuQmCC") right bottom var(--color-primary-light-7);
16+
background: var(--background-view-image);
1717
}
1818

1919
.image-diff-container .before-container {

‎web_src/css/repo/file-view.css‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@
8181
.view-raw img[src$=".svg" i] {
8282
max-height: 600px !important;
8383
max-width: 600px !important;
84+
background: var(--background-view-image);
8485
}
8586

8687
.file-view-render-container {

‎web_src/js/features/imagediff.ts‎

Lines changed: 111 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,33 @@ import {hideElem, loadElem, queryElemChildren, queryElems} from '../utils/dom.ts
33
import {parseDom} from '../utils.ts';
44
import {fomanticQuery} from '../modules/fomantic/base.ts';
55

6-
function getDefaultSvgBoundsIfUndefined(text: string, src: string) {
6+
type ImageContext = {
7+
imageBefore: HTMLImageElement | undefined,
8+
imageAfter: HTMLImageElement | undefined,
9+
sizeBefore: {width: number, height: number},
10+
sizeAfter: {width: number, height: number},
11+
maxSize: {width: number, height: number},
12+
ratio: [number, number, number, number],
13+
};
14+
15+
type ImageInfo = {
16+
path: string | null,
17+
mime: string | null,
18+
images: NodeListOf<HTMLImageElement>,
19+
boundsInfo: HTMLElement | null,
20+
};
21+
22+
type Bounds = {
23+
width: number,
24+
height: number,
25+
} | null;
26+
27+
type SvgBoundsInfo = {
28+
before: Bounds,
29+
after: Bounds,
30+
};
31+
32+
function getDefaultSvgBoundsIfUndefined(text: string, src: string): Bounds | null {
733
const defaultSize = 300;
834
const maxSize = 99999;
935

@@ -38,14 +64,14 @@ function getDefaultSvgBoundsIfUndefined(text: string, src: string) {
3864
return null;
3965
}
4066

41-
function createContext(imageAfter: HTMLImageElement, imageBefore: HTMLImageElement) {
67+
function createContext(imageAfter: HTMLImageElement, imageBefore: HTMLImageElement, svgBoundsInfo: SvgBoundsInfo): ImageContext {
4268
const sizeAfter = {
43-
width: imageAfter?.width || 0,
44-
height: imageAfter?.height || 0,
69+
width: svgBoundsInfo.after?.width || imageAfter?.width || 0,
70+
height: svgBoundsInfo.after?.height || imageAfter?.height || 0,
4571
};
4672
const sizeBefore = {
47-
width: imageBefore?.width || 0,
48-
height: imageBefore?.height || 0,
73+
width: svgBoundsInfo.before?.width || imageBefore?.width || 0,
74+
height: svgBoundsInfo.before?.height || imageBefore?.height || 0,
4975
};
5076
const maxSize = {
5177
width: Math.max(sizeBefore.width, sizeAfter.width),
@@ -80,7 +106,7 @@ class ImageDiff {
80106
// the container may be hidden by "viewed" checkbox, so use the parent's width for reference
81107
this.diffContainerWidth = Math.max(containerEl.closest('.diff-file-box')!.clientWidth - 300, 100);
82108

83-
const imageInfos = [{
109+
const imagePair: [ImageInfo, ImageInfo] = [{
84110
path: containerEl.getAttribute('data-path-after'),
85111
mime: containerEl.getAttribute('data-mime-after'),
86112
images: containerEl.querySelectorAll<HTMLImageElement>('img.image-after'), // matches 3 <img>
@@ -92,7 +118,8 @@ class ImageDiff {
92118
boundsInfo: containerEl.querySelector('.bounds-info-before'),
93119
}];
94120

95-
await Promise.all(imageInfos.map(async (info) => {
121+
const svgBoundsInfo: SvgBoundsInfo = {before: null, after: null};
122+
await Promise.all(imagePair.map(async (info, index) => {
96123
const [success] = await Promise.all(Array.from(info.images, (img) => {
97124
return loadElem(img, info.path!);
98125
}));
@@ -102,115 +129,112 @@ class ImageDiff {
102129
const resp = await GET(info.path!);
103130
const text = await resp.text();
104131
const bounds = getDefaultSvgBoundsIfUndefined(text, info.path!);
132+
svgBoundsInfo[index === 0 ? 'after' : 'before'] = bounds;
105133
if (bounds) {
106-
for (const el of info.images) {
107-
el.setAttribute('width', String(bounds.width));
108-
el.setAttribute('height', String(bounds.height));
109-
}
110134
hideElem(info.boundsInfo!);
111135
}
112136
}
113137
}));
114138

115-
const imagesAfter = imageInfos[0].images;
116-
const imagesBefore = imageInfos[1].images;
139+
const imagesAfter = imagePair[0].images;
140+
const imagesBefore = imagePair[1].images;
117141

118-
this.initSideBySide(createContext(imagesAfter[0], imagesBefore[0]));
142+
this.initSideBySide(createContext(imagesAfter[0], imagesBefore[0], svgBoundsInfo));
119143
if (imagesAfter.length > 0 && imagesBefore.length > 0) {
120-
this.initSwipe(createContext(imagesAfter[1], imagesBefore[1]));
121-
this.initOverlay(createContext(imagesAfter[2], imagesBefore[2]));
144+
this.initSwipe(createContext(imagesAfter[1], imagesBefore[1], svgBoundsInfo));
145+
this.initOverlay(createContext(imagesAfter[2], imagesBefore[2], svgBoundsInfo));
122146
}
123147
queryElemChildren(containerEl, '.image-diff-tabs', (el) => el.classList.remove('is-loading'));
124148
}
125149

126-
initSideBySide(sizes: Record<string, any>) {
150+
initSideBySide(ctx: ImageContext) {
127151
let factor = 1;
128-
if (sizes.maxSize.width > (this.diffContainerWidth - 24) / 2) {
129-
factor = (this.diffContainerWidth - 24) / 2 / sizes.maxSize.width;
152+
if (ctx.maxSize.width > (this.diffContainerWidth - 24) / 2) {
153+
factor = (this.diffContainerWidth - 24) / 2 / ctx.maxSize.width;
130154
}
131155

132-
const widthChanged = sizes.imageAfter && sizes.imageBefore && sizes.imageAfter.naturalWidth !== sizes.imageBefore.naturalWidth;
133-
const heightChanged = sizes.imageAfter && sizes.imageBefore && sizes.imageAfter.naturalHeight !== sizes.imageBefore.naturalHeight;
134-
if (sizes.imageAfter) {
156+
const widthChanged = ctx.imageAfter && ctx.imageBefore && ctx.imageAfter.naturalWidth !== ctx.imageBefore.naturalWidth;
157+
const heightChanged = ctx.imageAfter && ctx.imageBefore && ctx.imageAfter.naturalHeight !== ctx.imageBefore.naturalHeight;
158+
if (ctx.imageAfter) {
135159
const boundsInfoAfterWidth = this.containerEl.querySelector('.bounds-info-after .bounds-info-width');
136160
if (boundsInfoAfterWidth) {
137-
boundsInfoAfterWidth.textContent = `${sizes.imageAfter.naturalWidth}px`;
161+
boundsInfoAfterWidth.textContent = `${ctx.imageAfter.naturalWidth}px`;
138162
boundsInfoAfterWidth.classList.toggle('green', widthChanged);
139163
}
140164
const boundsInfoAfterHeight = this.containerEl.querySelector('.bounds-info-after .bounds-info-height');
141165
if (boundsInfoAfterHeight) {
142-
boundsInfoAfterHeight.textContent = `${sizes.imageAfter.naturalHeight}px`;
166+
boundsInfoAfterHeight.textContent = `${ctx.imageAfter.naturalHeight}px`;
143167
boundsInfoAfterHeight.classList.toggle('green', heightChanged);
144168
}
145169
}
146170

147-
if (sizes.imageBefore) {
171+
if (ctx.imageBefore) {
148172
const boundsInfoBeforeWidth = this.containerEl.querySelector('.bounds-info-before .bounds-info-width');
149173
if (boundsInfoBeforeWidth) {
150-
boundsInfoBeforeWidth.textContent = `${sizes.imageBefore.naturalWidth}px`;
174+
boundsInfoBeforeWidth.textContent = `${ctx.imageBefore.naturalWidth}px`;
151175
boundsInfoBeforeWidth.classList.toggle('red', widthChanged);
152176
}
153177
const boundsInfoBeforeHeight = this.containerEl.querySelector('.bounds-info-before .bounds-info-height');
154178
if (boundsInfoBeforeHeight) {
155-
boundsInfoBeforeHeight.textContent = `${sizes.imageBefore.naturalHeight}px`;
179+
boundsInfoBeforeHeight.textContent = `${ctx.imageBefore.naturalHeight}px`;
156180
boundsInfoBeforeHeight.classList.toggle('red', heightChanged);
157181
}
158182
}
159183

160-
if (sizes.imageAfter) {
161-
const container = sizes.imageAfter.parentNode;
162-
sizes.imageAfter.style.width = `${sizes.sizeAfter.width * factor}px`;
163-
sizes.imageAfter.style.height = `${sizes.sizeAfter.height * factor}px`;
184+
if (ctx.imageAfter) {
185+
const container = ctx.imageAfter.parentNode as HTMLElement;
186+
ctx.imageAfter.style.width = `${ctx.sizeAfter.width * factor}px`;
187+
ctx.imageAfter.style.height = `${ctx.sizeAfter.height * factor}px`;
164188
container.style.margin = '10px auto';
165-
container.style.width = `${sizes.sizeAfter.width * factor + 2}px`;
166-
container.style.height = `${sizes.sizeAfter.height * factor + 2}px`;
189+
container.style.width = `${ctx.sizeAfter.width * factor + 2}px`;
190+
container.style.height = `${ctx.sizeAfter.height * factor + 2}px`;
167191
}
168192

169-
if (sizes.imageBefore) {
170-
const container = sizes.imageBefore.parentNode;
171-
sizes.imageBefore.style.width = `${sizes.sizeBefore.width * factor}px`;
172-
sizes.imageBefore.style.height = `${sizes.sizeBefore.height * factor}px`;
193+
if (ctx.imageBefore) {
194+
const container = ctx.imageBefore.parentNode as HTMLElement;
195+
ctx.imageBefore.style.width = `${ctx.sizeBefore.width * factor}px`;
196+
ctx.imageBefore.style.height = `${ctx.sizeBefore.height * factor}px`;
173197
container.style.margin = '10px auto';
174-
container.style.width = `${sizes.sizeBefore.width * factor + 2}px`;
175-
container.style.height = `${sizes.sizeBefore.height * factor + 2}px`;
198+
container.style.width = `${ctx.sizeBefore.width * factor + 2}px`;
199+
container.style.height = `${ctx.sizeBefore.height * factor + 2}px`;
176200
}
177201
}
178202

179-
initSwipe(sizes: Record<string, any>) {
203+
initSwipe(ctx: ImageContext) {
180204
let factor = 1;
181-
if (sizes.maxSize.width > this.diffContainerWidth - 12) {
182-
factor = (this.diffContainerWidth - 12) / sizes.maxSize.width;
205+
if (ctx.maxSize.width > this.diffContainerWidth - 12) {
206+
factor = (this.diffContainerWidth - 12) / ctx.maxSize.width;
183207
}
184208

185-
if (sizes.imageAfter) {
186-
const imgParent = sizes.imageAfter.parentNode;
187-
const swipeFrame = imgParent.parentNode;
188-
sizes.imageAfter.style.width = `${sizes.sizeAfter.width * factor}px`;
189-
sizes.imageAfter.style.height = `${sizes.sizeAfter.height * factor}px`;
190-
imgParent.style.margin = `0px ${sizes.ratio[0] * factor}px`;
191-
imgParent.style.width = `${sizes.sizeAfter.width * factor + 2}px`;
192-
imgParent.style.height = `${sizes.sizeAfter.height * factor + 2}px`;
193-
swipeFrame.style.padding = `${sizes.ratio[1] * factor}px 0 0 0`;
194-
swipeFrame.style.width = `${sizes.maxSize.width * factor + 2}px`;
209+
if (ctx.imageAfter) {
210+
const imgParent = ctx.imageAfter.parentNode as HTMLElement;
211+
const swipeFrame = imgParent.parentNode as HTMLElement;
212+
ctx.imageAfter.style.width = `${ctx.sizeAfter.width * factor}px`;
213+
ctx.imageAfter.style.height = `${ctx.sizeAfter.height * factor}px`;
214+
imgParent.style.margin = `0px ${ctx.ratio[0] * factor}px`;
215+
imgParent.style.width = `${ctx.sizeAfter.width * factor + 2}px`;
216+
imgParent.style.height = `${ctx.sizeAfter.height * factor + 2}px`;
217+
swipeFrame.style.padding = `${ctx.ratio[1] * factor}px 0 0 0`;
218+
swipeFrame.style.width = `${ctx.maxSize.width * factor + 2}px`;
195219
}
196220

197-
if (sizes.imageBefore) {
198-
const imgParent = sizes.imageBefore.parentNode;
199-
const swipeFrame = imgParent.parentNode;
200-
sizes.imageBefore.style.width = `${sizes.sizeBefore.width * factor}px`;
201-
sizes.imageBefore.style.height = `${sizes.sizeBefore.height * factor}px`;
202-
imgParent.style.margin = `${sizes.ratio[3] * factor}px ${sizes.ratio[2] * factor}px`;
203-
imgParent.style.width = `${sizes.sizeBefore.width * factor + 2}px`;
204-
imgParent.style.height = `${sizes.sizeBefore.height * factor + 2}px`;
205-
swipeFrame.style.width = `${sizes.maxSize.width * factor + 2}px`;
206-
swipeFrame.style.height = `${sizes.maxSize.height * factor + 2}px`;
221+
if (ctx.imageBefore) {
222+
const imgParent = ctx.imageBefore.parentNode as HTMLElement;
223+
const swipeFrame = imgParent.parentNode as HTMLElement;
224+
ctx.imageBefore.style.width = `${ctx.sizeBefore.width * factor}px`;
225+
ctx.imageBefore.style.height = `${ctx.sizeBefore.height * factor}px`;
226+
imgParent.style.margin = `${ctx.ratio[3] * factor}px ${ctx.ratio[2] * factor}px`;
227+
imgParent.style.width = `${ctx.sizeBefore.width * factor + 2}px`;
228+
imgParent.style.height = `${ctx.sizeBefore.height * factor + 2}px`;
229+
swipeFrame.style.width = `${ctx.maxSize.width * factor + 2}px`;
230+
swipeFrame.style.height = `${ctx.maxSize.height * factor + 2}px`;
207231
}
208232

209233
// extra height for inner "position: absolute" elements
210234
const swipe = this.containerEl.querySelector<HTMLElement>('.diff-swipe');
211235
if (swipe) {
212-
swipe.style.width = `${sizes.maxSize.width * factor + 2}px`;
213-
swipe.style.height = `${sizes.maxSize.height * factor + 30}px`;
236+
swipe.style.width = `${ctx.maxSize.width * factor + 2}px`;
237+
swipe.style.height = `${ctx.maxSize.height * factor + 30}px`;
214238
}
215239

216240
this.containerEl.querySelector('.swipe-bar')!.addEventListener('mousedown', (e) => {
@@ -237,40 +261,40 @@ class ImageDiff {
237261
document.addEventListener('mouseup', removeEventListeners);
238262
}
239263

240-
initOverlay(sizes: Record<string, any>) {
264+
initOverlay(ctx: ImageContext) {
241265
let factor = 1;
242-
if (sizes.maxSize.width > this.diffContainerWidth - 12) {
243-
factor = (this.diffContainerWidth - 12) / sizes.maxSize.width;
266+
if (ctx.maxSize.width > this.diffContainerWidth - 12) {
267+
factor = (this.diffContainerWidth - 12) / ctx.maxSize.width;
244268
}
245269

246-
if (sizes.imageAfter) {
247-
const container = sizes.imageAfter.parentNode;
248-
sizes.imageAfter.style.width = `${sizes.sizeAfter.width * factor}px`;
249-
sizes.imageAfter.style.height = `${sizes.sizeAfter.height * factor}px`;
250-
container.style.margin = `${sizes.ratio[1] * factor}px ${sizes.ratio[0] * factor}px`;
251-
container.style.width = `${sizes.sizeAfter.width * factor + 2}px`;
252-
container.style.height = `${sizes.sizeAfter.height * factor + 2}px`;
270+
if (ctx.imageAfter) {
271+
const container = ctx.imageAfter.parentNode as HTMLElement;
272+
ctx.imageAfter.style.width = `${ctx.sizeAfter.width * factor}px`;
273+
ctx.imageAfter.style.height = `${ctx.sizeAfter.height * factor}px`;
274+
container.style.margin = `${ctx.ratio[1] * factor}px ${ctx.ratio[0] * factor}px`;
275+
container.style.width = `${ctx.sizeAfter.width * factor + 2}px`;
276+
container.style.height = `${ctx.sizeAfter.height * factor + 2}px`;
253277
}
254278

255-
if (sizes.imageBefore) {
256-
const container = sizes.imageBefore.parentNode;
257-
const overlayFrame = container.parentNode;
258-
sizes.imageBefore.style.width = `${sizes.sizeBefore.width * factor}px`;
259-
sizes.imageBefore.style.height = `${sizes.sizeBefore.height * factor}px`;
260-
container.style.margin = `${sizes.ratio[3] * factor}px ${sizes.ratio[2] * factor}px`;
261-
container.style.width = `${sizes.sizeBefore.width * factor + 2}px`;
262-
container.style.height = `${sizes.sizeBefore.height * factor + 2}px`;
279+
if (ctx.imageBefore) {
280+
const container = ctx.imageBefore.parentNode as HTMLElement;
281+
const overlayFrame = container.parentNode as HTMLElement;
282+
ctx.imageBefore.style.width = `${ctx.sizeBefore.width * factor}px`;
283+
ctx.imageBefore.style.height = `${ctx.sizeBefore.height * factor}px`;
284+
container.style.margin = `${ctx.ratio[3] * factor}px ${ctx.ratio[2] * factor}px`;
285+
container.style.width = `${ctx.sizeBefore.width * factor + 2}px`;
286+
container.style.height = `${ctx.sizeBefore.height * factor + 2}px`;
263287

264288
// some inner elements are `position: absolute`, so the container's height must be large enough
265-
overlayFrame.style.width = `${sizes.maxSize.width * factor + 2}px`;
266-
overlayFrame.style.height = `${sizes.maxSize.height * factor + 2}px`;
289+
overlayFrame.style.width = `${ctx.maxSize.width * factor + 2}px`;
290+
overlayFrame.style.height = `${ctx.maxSize.height * factor + 2}px`;
267291
}
268292

269293
const rangeInput = this.containerEl.querySelector<HTMLInputElement>('input[type="range"]')!;
270294

271295
function updateOpacity() {
272-
if (sizes.imageAfter) {
273-
sizes.imageAfter.parentNode.style.opacity = `${Number(rangeInput.value) / 100}`;
296+
if (ctx.imageAfter) {
297+
(ctx.imageAfter.parentNode as HTMLElement).style.opacity = `${Number(rangeInput.value) / 100}`;
274298
}
275299
}
276300

0 commit comments

Comments
 (0)