Skip to content

Replace ajax with fetch, improve image diff #27267

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 29 commits into from
Oct 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
aeb15d9
Replace two cases of jQuery ajax with fetch
silverwind Sep 25, 2023
0290d7d
use shared xml functions
silverwind Sep 25, 2023
71d0e18
Merge branch 'main' into fetch1
silverwind Sep 29, 2023
a4717dd
rename to parseDom
silverwind Sep 29, 2023
18c0b39
rename vars
silverwind Sep 29, 2023
b4217d4
remove comment
silverwind Sep 29, 2023
8039365
make SniffedType accessible in diff
silverwind Oct 2, 2023
9ae1c4a
propagate mime down to javascript
silverwind Oct 2, 2023
69d41a2
fix lint
silverwind Oct 2, 2023
34a862c
use previous function naming style
silverwind Oct 2, 2023
eb064c4
fix name
silverwind Oct 2, 2023
94a6485
fmt
silverwind Oct 2, 2023
c83a48e
rewrite image loading
silverwind Oct 2, 2023
e967cb4
invert return value
silverwind Oct 2, 2023
f5f72f4
move loadImage into utils/dom.js
silverwind Oct 2, 2023
e42dfb8
improve doc
silverwind Oct 2, 2023
c6ac19e
rename to loadElement, remove unnecessary target check
silverwind Oct 2, 2023
f996bc5
doc
silverwind Oct 2, 2023
acacbca
rename
silverwind Oct 2, 2023
4c0212a
rename vars to sniffedType
silverwind Oct 3, 2023
cc8721f
always call handleSvgSize, inline handleSvgSize again
silverwind Oct 3, 2023
866f982
Merge branch 'main' into fetch1
silverwind Oct 3, 2023
31a7de8
use regular comment and add mdn link
silverwind Oct 3, 2023
126c027
small refactor
silverwind Oct 3, 2023
6c3544c
small refactor in request
silverwind Oct 3, 2023
44646eb
Merge branch 'main' into fetch1
silverwind Oct 9, 2023
12b49b5
remove dead condition
silverwind Oct 9, 2023
06c5496
Merge branch 'main' into fetch1
GiteaBot Oct 11, 2023
d34ba3a
use once parameter on addEventListener
silverwind Oct 11, 2023
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
27 changes: 17 additions & 10 deletions routers/web/repo/compare.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
"code.gitea.io/gitea/modules/markup"
"code.gitea.io/gitea/modules/setting"
api "code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/modules/typesniffer"
"code.gitea.io/gitea/modules/upload"
"code.gitea.io/gitea/modules/util"
"code.gitea.io/gitea/services/gitdiff"
Expand Down Expand Up @@ -60,6 +61,21 @@ func setCompareContext(ctx *context.Context, before, head *git.Commit, headOwner
return blob
}

ctx.Data["GetSniffedTypeForBlob"] = func(blob *git.Blob) typesniffer.SniffedType {
st := typesniffer.SniffedType{}

if blob == nil {
return st
}

st, err := blob.GuessContentType()
if err != nil {
log.Error("GuessContentType failed: %v", err)
return st
}
return st
}

setPathsCompareContext(ctx, before, head, headOwner, headName)
setImageCompareContext(ctx)
setCsvCompareContext(ctx)
Expand Down Expand Up @@ -87,16 +103,7 @@ func setPathsCompareContext(ctx *context.Context, base, head *git.Commit, headOw

// setImageCompareContext sets context data that is required by image compare template
func setImageCompareContext(ctx *context.Context) {
ctx.Data["IsBlobAnImage"] = func(blob *git.Blob) bool {
if blob == nil {
return false
}

st, err := blob.GuessContentType()
if err != nil {
log.Error("GuessContentType failed: %v", err)
return false
}
ctx.Data["IsSniffedTypeAnImage"] = func(st typesniffer.SniffedType) bool {
return st.IsImage() && (setting.UI.SVG.Enabled || !st.IsSvgImage())
}
}
Expand Down
8 changes: 5 additions & 3 deletions templates/repo/diff/box.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,9 @@
{{/*notice: the index of Diff.Files should not be used for element ID, because the index will be restarted from 0 when doing load-more for PRs with a lot of files*/}}
{{$blobBase := call $.GetBlobByPathForCommit $.BeforeCommit $file.OldName}}
{{$blobHead := call $.GetBlobByPathForCommit $.HeadCommit $file.Name}}
{{$isImage := or (call $.IsBlobAnImage $blobBase) (call $.IsBlobAnImage $blobHead)}}
{{$sniffedTypeBase := call $.GetSniffedTypeForBlob $blobBase}}
{{$sniffedTypeHead := call $.GetSniffedTypeForBlob $blobHead}}
{{$isImage:= or (call $.IsSniffedTypeAnImage $sniffedTypeBase) (call $.IsSniffedTypeAnImage $sniffedTypeHead)}}
{{$isCsv := (call $.IsCsvFile $file)}}
{{$showFileViewToggle := or $isImage (and (not $file.IsIncomplete) $isCsv)}}
{{$isExpandable := or (gt $file.Addition 0) (gt $file.Deletion 0) $file.IsBin}}
Expand Down Expand Up @@ -198,9 +200,9 @@
<div id="diff-rendered-{{$file.NameHash}}" class="file-body file-code {{if $.IsSplitStyle}}code-diff-split{{else}}code-diff-unified{{end}} gt-overflow-x-scroll">
<table class="chroma gt-w-100">
{{if $isImage}}
{{template "repo/diff/image_diff" dict "file" . "root" $ "blobBase" $blobBase "blobHead" $blobHead}}
{{template "repo/diff/image_diff" dict "file" . "root" $ "blobBase" $blobBase "blobHead" $blobHead "sniffedTypeBase" $sniffedTypeBase "sniffedTypeHead" $sniffedTypeHead}}
{{else}}
{{template "repo/diff/csv_diff" dict "file" . "root" $ "blobBase" $blobBase "blobHead" $blobHead}}
{{template "repo/diff/csv_diff" dict "file" . "root" $ "blobBase" $blobBase "blobHead" $blobHead "sniffedTypeBase" $sniffedTypeBase "sniffedTypeHead" $sniffedTypeHead}}
{{end}}
</table>
</div>
Expand Down
7 changes: 6 additions & 1 deletion templates/repo/diff/image_diff.tmpl
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
{{if or .blobBase .blobHead}}
<tr>
<td colspan="2">
<div class="image-diff" data-path-before="{{.root.BeforeRawPath}}/{{PathEscapeSegments .file.OldName}}" data-path-after="{{.root.RawPath}}/{{PathEscapeSegments .file.Name}}">
<div class="image-diff"
data-path-before="{{.root.BeforeRawPath}}/{{PathEscapeSegments .file.OldName}}"
data-path-after="{{.root.RawPath}}/{{PathEscapeSegments .file.Name}}"
data-mime-before="{{.sniffedTypeBase.GetMimeType}}"
data-mime-after="{{.sniffedTypeHead.GetMimeType}}"
>
<div class="ui secondary pointing tabular top attached borderless menu new-menu">
<div class="new-menu-inner">
<a class="item active" data-tab="diff-side-by-side-{{.file.Index}}">{{ctx.Locale.Tr "repo.diff.image.side_by_side"}}</a>
Expand Down
7 changes: 3 additions & 4 deletions web_src/js/features/common-global.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {htmlEscape} from 'escape-goat';
import {showTemporaryTooltip} from '../modules/tippy.js';
import {confirmModal} from './comp/ConfirmModal.js';
import {showErrorToast} from '../modules/toast.js';
import {request} from '../modules/fetch.js';
import {request, POST} from '../modules/fetch.js';

const {appUrl, appSubUrl, csrfToken, i18n} = window.config;

Expand Down Expand Up @@ -243,9 +243,8 @@ export function initGlobalDropzone() {
this.on('removedfile', (file) => {
$(`#${file.uuid}`).remove();
if ($dropzone.data('remove-url')) {
$.post($dropzone.data('remove-url'), {
file: file.uuid,
_csrf: csrfToken,
POST($dropzone.data('remove-url'), {
data: new URLSearchParams({file: file.uuid}),
});
}
});
Expand Down
91 changes: 37 additions & 54 deletions web_src/js/features/imagediff.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
import $ from 'jquery';
import {hideElem} from '../utils/dom.js';
import {GET} from '../modules/fetch.js';
import {hideElem, loadElem} from '../utils/dom.js';
import {parseDom} from '../utils.js';

function getDefaultSvgBoundsIfUndefined(svgXml, src) {
function getDefaultSvgBoundsIfUndefined(text, src) {
const DefaultSize = 300;
const MaxSize = 99999;

const svg = svgXml.documentElement;
const svgDoc = parseDom(text, 'image/svg+xml');
const svg = svgDoc.documentElement;
const width = svg?.width?.baseVal;
const height = svg?.height?.baseVal;
if (width === undefined || height === undefined) {
Expand Down Expand Up @@ -65,74 +68,54 @@ export function initImageDiff() {
};
}

$('.image-diff:not([data-image-diff-loaded])').each(function() {
$('.image-diff:not([data-image-diff-loaded])').each(async function() {
const $container = $(this);
$container.attr('data-image-diff-loaded', 'true');

// the container may be hidden by "viewed" checkbox, so use the parent's width for reference
const diffContainerWidth = Math.max($container.closest('.diff-file-box').width() - 300, 100);
const pathAfter = $container.data('path-after');
const pathBefore = $container.data('path-before');

const imageInfos = [{
loaded: false,
path: pathAfter,
$image: $container.find('img.image-after'),
path: this.getAttribute('data-path-after'),
mime: this.getAttribute('data-mime-after'),
$images: $container.find('img.image-after'), // matches 3 <img>
$boundsInfo: $container.find('.bounds-info-after')
}, {
loaded: false,
path: pathBefore,
$image: $container.find('img.image-before'),
path: this.getAttribute('data-path-before'),
mime: this.getAttribute('data-mime-before'),
$images: $container.find('img.image-before'), // matches 3 <img>
$boundsInfo: $container.find('.bounds-info-before')
}];

for (const info of imageInfos) {
if (info.$image.length > 0) {
$.ajax({
url: info.path,
success: (data, _, jqXHR) => {
info.$image.on('load', () => {
info.loaded = true;
setReadyIfLoaded();
}).on('error', () => {
info.loaded = true;
setReadyIfLoaded();
info.$boundsInfo.text('(image error)');
});
info.$image.attr('src', info.path);

if (jqXHR.getResponseHeader('Content-Type') === 'image/svg+xml') {
const bounds = getDefaultSvgBoundsIfUndefined(data, info.path);
if (bounds) {
info.$image.attr('width', bounds.width);
info.$image.attr('height', bounds.height);
hideElem(info.$boundsInfo);
}
}
}
});
} else {
info.loaded = true;
setReadyIfLoaded();
}
}

function setReadyIfLoaded() {
if (imageInfos[0].loaded && imageInfos[1].loaded) {
initViews(imageInfos[0].$image, imageInfos[1].$image);
await Promise.all(imageInfos.map(async (info) => {
const [success] = await Promise.all(Array.from(info.$images, (img) => {
return loadElem(img, info.path);
}));
// only the first images is associated with $boundsInfo
if (!success) info.$boundsInfo.text('(image error)');
if (info.mime === 'image/svg+xml') {
const resp = await GET(info.path);
const text = await resp.text();
const bounds = getDefaultSvgBoundsIfUndefined(text, info.path);
if (bounds) {
info.$images.attr('width', bounds.width);
info.$images.attr('height', bounds.height);
hideElem(info.$boundsInfo);
}
}
}
}));

function initViews($imageAfter, $imageBefore) {
initSideBySide(createContext($imageAfter[0], $imageBefore[0]));
if ($imageAfter.length > 0 && $imageBefore.length > 0) {
initSwipe(createContext($imageAfter[1], $imageBefore[1]));
initOverlay(createContext($imageAfter[2], $imageBefore[2]));
}
const $imagesAfter = imageInfos[0].$images;
const $imagesBefore = imageInfos[1].$images;

$container.find('> .image-diff-tabs').removeClass('is-loading');
initSideBySide(createContext($imagesAfter[0], $imagesBefore[0]));
if ($imagesAfter.length > 0 && $imagesBefore.length > 0) {
initSwipe(createContext($imagesAfter[1], $imagesBefore[1]));
initOverlay(createContext($imagesAfter[2], $imagesBefore[2]));
}

$container.find('> .image-diff-tabs').removeClass('is-loading');

function initSideBySide(sizes) {
let factor = 1;
if (sizes.max.width > (diffContainerWidth - 24) / 2) {
Expand Down
4 changes: 1 addition & 3 deletions web_src/js/modules/fetch.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,7 @@ const safeMethods = new Set(['GET', 'HEAD', 'OPTIONS', 'TRACE']);
export function request(url, {method = 'GET', headers = {}, data, body, ...other} = {}) {
let contentType;
if (!body) {
if (data instanceof FormData) {
body = data;
} else if (data instanceof URLSearchParams) {
if (data instanceof FormData || data instanceof URLSearchParams) {
body = data;
} else if (isObject(data) || Array.isArray(data)) {
contentType = 'application/json';
Expand Down
10 changes: 4 additions & 6 deletions web_src/js/svg.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {h} from 'vue';
import {parseDom, serializeXml} from './utils.js';
import giteaDoubleChevronLeft from '../../public/assets/img/svg/gitea-double-chevron-left.svg';
import giteaDoubleChevronRight from '../../public/assets/img/svg/gitea-double-chevron-right.svg';
import giteaEmptyCheckbox from '../../public/assets/img/svg/gitea-empty-checkbox.svg';
Expand Down Expand Up @@ -145,22 +146,19 @@ const svgs = {
// At the moment, developers must check, pick and fill the names manually,
// most of the SVG icons in assets couldn't be used directly.

const parser = new DOMParser();
const serializer = new XMLSerializer();

// retrieve an HTML string for given SVG icon name, size and additional classes
export function svg(name, size = 16, className = '') {
if (!(name in svgs)) throw new Error(`Unknown SVG icon: ${name}`);
if (size === 16 && !className) return svgs[name];

const document = parser.parseFromString(svgs[name], 'image/svg+xml');
const document = parseDom(svgs[name], 'image/svg+xml');
const svgNode = document.firstChild;
if (size !== 16) {
svgNode.setAttribute('width', String(size));
svgNode.setAttribute('height', String(size));
}
if (className) svgNode.classList.add(...className.split(/\s+/).filter(Boolean));
return serializer.serializeToString(svgNode);
return serializeXml(svgNode);
}

export function svgParseOuterInner(name) {
Expand All @@ -176,7 +174,7 @@ export function svgParseOuterInner(name) {
if (p1 === -1 || p2 === -1) throw new Error(`Invalid SVG icon: ${name}`);
const svgInnerHtml = svgStr.slice(p1 + 1, p2);
const svgOuterHtml = svgStr.slice(0, p1 + 1) + svgStr.slice(p2);
const svgDoc = parser.parseFromString(svgOuterHtml, 'image/svg+xml');
const svgDoc = parseDom(svgOuterHtml, 'image/svg+xml');
const svgOuter = svgDoc.firstChild;
return {svgOuter, svgInnerHtml};
}
Expand Down
11 changes: 11 additions & 0 deletions web_src/js/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -128,3 +128,14 @@ export function decodeURLEncodedBase64(base64url) {
.replace(/_/g, '/')
.replace(/-/g, '+'));
}

const domParser = new DOMParser();
const xmlSerializer = new XMLSerializer();

export function parseDom(text, contentType) {
return domParser.parseFromString(text, contentType);
}

export function serializeXml(node) {
return xmlSerializer.serializeToString(node);
}
11 changes: 11 additions & 0 deletions web_src/js/utils/dom.js
Original file line number Diff line number Diff line change
Expand Up @@ -183,3 +183,14 @@ export function autosize(textarea, {viewportMarginBottom = 0} = {}) {
export function onInputDebounce(fn) {
return debounce(300, fn);
}

// Set the `src` attribute on an element and returns a promise that resolves once the element
// has loaded or errored. Suitable for all elements mention in:
// https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/load_event
export function loadElem(el, src) {
return new Promise((resolve) => {
el.addEventListener('load', () => resolve(true), {once: true});
el.addEventListener('error', () => resolve(false), {once: true});
el.src = src;
});
}