Skip to content

Replace ajax with fetch, improve image diff (#27267) #27583

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 1 commit into from
Oct 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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;
});
}