Skip to content

Commit

Permalink
chore(ui): improve icon fetching. WF-141
Browse files Browse the repository at this point in the history
  • Loading branch information
madeindjs committed Jan 8, 2025
1 parent 262a521 commit 526bdaa
Show file tree
Hide file tree
Showing 9 changed files with 219 additions and 52 deletions.
33 changes: 11 additions & 22 deletions src/ui/src/builder/sidebar/BuilderSidebarToolkit.vue
Original file line number Diff line number Diff line change
Expand Up @@ -23,19 +23,10 @@
@dragend="handleDragEnd($event)"
@dragstart="handleDragStart($event, tool.type)"
>
<img
<SharedImgWithFallback
:alt="`(Icon for ${tool.name})`"
:src="`./../../../../components/${tool.type}.svg`"
draggable="false"
@error="
!isImageFallback[tool.type]
? handleImageError(
$event,
tool.type,
tool.category,
)
: undefined
"
:urls="getToolIcons(tool)"
/>
<div class="name">{{ tool.name }}</div>
</div>
Expand All @@ -54,12 +45,13 @@ import {
import injectionKeys from "@/injectionKeys";
import { useDragDropComponent } from "../useDragDropComponent";
import { Component } from "@/writerTypes";
import SharedImgWithFallback from "@/components/shared/SharedImgWithFallback.vue";
import { convertAbsolutePathtoFullURL } from "@/utils/url";
const wf = inject(injectionKeys.core);
const wfbm = inject(injectionKeys.builderManager);
const { removeInsertionCandidacy } = useDragDropComponent(wf);
const query = ref("");
const isImageFallback = ref<Record<Component["type"], boolean>>({});
const displayedCategories = [
"Layout",
Expand Down Expand Up @@ -114,16 +106,6 @@ function getRelevantToolsInCategory(categoryId: string) {
return queryApplied;
}
function handleImageError(
ev: Event,
type: Component["type"],
categoryId: string,
) {
isImageFallback.value[type] = true; // Prevent calling more than once
const imageEl = ev.target as HTMLImageElement;
imageEl.src = `./../../../../components/category_${categoryId}.svg`;
}
function handleDragStart(ev: DragEvent, type: Component["type"]) {
wfbm.setSelection(null);
ev.dataTransfer.setData(`application/json;writer=${type},`, "{}");
Expand All @@ -133,6 +115,13 @@ function handleDragEnd(ev: DragEvent) {
removeInsertionCandidacy(ev);
}
function getToolIcons(tool: ReturnType<typeof getRelevantToolsInCategory>[0]) {
return [
`/components/${tool.type}.svg`,
`/components/category_${tool.category}.svg`,
].map((p) => convertAbsolutePathtoFullURL(p));
}
watch(activeToolkit, () => {
query.value = "";
});
Expand Down
2 changes: 1 addition & 1 deletion src/ui/src/components/core/content/CoreChatbot.vue
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,7 @@ function scrollToBottom() {
}
const encodeFile = async (file: File) => {
var reader = new FileReader();
const reader = new FileReader();
reader.readAsDataURL(file);
return new Promise((resolve, reject) => {
Expand Down
37 changes: 37 additions & 0 deletions src/ui/src/components/shared/SharedImgWithFallback.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import { describe, it, expect, vi, beforeEach, Mock } from "vitest";
import SharedImgWithFallback from "./SharedImgWithFallback.vue";
import { flushPromises, shallowMount } from "@vue/test-utils";

describe("SharedImgWithFallback", () => {
let fetch: Mock;

beforeEach(() => {
fetch = vi.fn().mockResolvedValue({
ok: true,
headers: new Map([["Content-Type", "image/png"]]),
});
global.fetch = fetch;
});

it("should use the last image because the first two are not valid", async () => {
fetch
.mockRejectedValueOnce(new Error())
.mockResolvedValueOnce({
ok: true,
headers: new Map([["Content-Type", "text/html"]]),
})
.mockResolvedValue({
ok: true,
headers: new Map([["Content-Type", "image/png"]]),
});

const wrapper = shallowMount(SharedImgWithFallback, {
props: { urls: ["/img1.svg", "/img2.svg", "/img3.svg"] },
});
expect(wrapper.get("img").attributes().src).toBe("");

await flushPromises();

expect(wrapper.get("img").attributes().src).toBe("/img3.svg");
});
});
31 changes: 31 additions & 0 deletions src/ui/src/components/shared/SharedImgWithFallback.vue
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<template>
<img :src="src" />
</template>

<script lang="ts" setup>
import { useAssetContentType } from "@/composables/useAssetContentType";
import { PropType, ref, toRef, watch } from "vue";
const props = defineProps({
urls: { type: Array as PropType<string[]>, required: true },
});
const src = ref("");
const { fetchAssetContentType } = useAssetContentType();
watch(
toRef(props, "urls"),
async (urls) => {
src.value = "";
for (const url of urls) {
const contentType = await fetchAssetContentType(url);
// ensure that the content type is valid and not HTML (the server can responds with a default HTML page)
if (!contentType || contentType === "text/html") continue;
return (src.value = url);
}
},
{ immediate: true },
);
</script>
38 changes: 9 additions & 29 deletions src/ui/src/components/workflows/abstract/WorkflowsNode.vue
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<template>
<div class="WorkflowsNode">
<div class="title">
<img :src="imagePath" />
<SharedImgWithFallback :urls="possibleImageUrls" />
<WorkflowsNodeNamer
:component-id="componentId"
class="nodeNamer"
Expand Down Expand Up @@ -69,17 +69,18 @@ export default {
};
</script>
<script setup lang="ts">
import { computed, inject, onMounted, ref, watch } from "vue";
import { computed, inject, watch } from "vue";
import injectionKeys from "@/injectionKeys";
import { FieldType, WriterComponentDefinition } from "@/writerTypes";
import WorkflowsNodeNamer from "../base/WorkflowsNodeNamer.vue";
import SharedImgWithFallback from "@/components/shared/SharedImgWithFallback.vue";
import { convertAbsolutePathtoFullURL } from "@/utils/url";
const emit = defineEmits(["outMousedown", "engaged"]);
const wf = inject(injectionKeys.core);
const wfbm = inject(injectionKeys.builderManager);
const componentId = inject(injectionKeys.componentId);
const fields = inject(injectionKeys.evaluatedFields);
const imagePath = ref<string>(null);
const component = computed(() => {
const component = wf.getComponentById(componentId);
Expand Down Expand Up @@ -142,38 +143,17 @@ function handleOutMousedown(ev: DragEvent, outId: string) {
emit("outMousedown", outId);
}
async function checkIfUrlExists(url: string) {
try {
const response = await fetch(url, { method: "HEAD" });
return response.ok;
} catch {
return false;
}
}
async function getBestAvailableImagePath() {
const possibleImageUrls = computed(() => {
const paths = [
`./../../../../components/${component.value.type}.svg`,
`./../../../../components/workflows_category_${def.value.category}.svg`,
`/components/${component.value.type}.svg`,
`/components/workflows_category_${def.value.category}.svg`,
];
if (wf.featureFlags.value.includes("custom_block_icons")) {
paths.unshift(
`./../../../../static/components/${component.value.id}.svg`,
);
paths.unshift(`/static/components/${component.value.id}.svg`);
}
for (let i = 0; i < paths.length; i++) {
const path = paths[i];
if (await checkIfUrlExists(path)) {
return path;
}
}
return "";
}
onMounted(async () => {
imagePath.value = await getBestAvailableImagePath();
return paths.map((p) => convertAbsolutePathtoFullURL(p));
});
watch(isEngaged, () => {
Expand Down
65 changes: 65 additions & 0 deletions src/ui/src/composables/useAssetContentType.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import { useAssetContentType } from "./useAssetContentType";
import { beforeEach, describe, it, expect, Mock, vi } from "vitest";

describe(useAssetContentType.name, () => {
let fetch: Mock;

beforeEach(() => {
fetch = vi.fn().mockResolvedValue({
ok: true,
headers: new Map([["Content-Type", "image/png"]]),
});
global.fetch = fetch;

useAssetContentType().clearCache();
});

it("should handle error ", async () => {
fetch.mockRejectedValue(new Error());
const { fetchAssetContentType } = useAssetContentType();

expect(await fetchAssetContentType("https://test.com")).toBeUndefined();
expect(fetch).toHaveBeenCalledOnce();
});

it("should cache fetch call in sequential calls", async () => {
const { fetchAssetContentType } = useAssetContentType();

expect(await fetchAssetContentType("https://test.com")).toBe(
"image/png",
);
expect(await fetchAssetContentType("https://test.com")).toBe(
"image/png",
);
expect(fetch).toHaveBeenCalledOnce();
});

it("should cache fetch call in parrallel call", async () => {
vi.useFakeTimers();

fetch.mockResolvedValue(
new Promise((res) =>
setTimeout(
() =>
res({
ok: true,
headers: new Map([["Content-Type", "image/png"]]),
}),
3_000,
),
),
);

const { fetchAssetContentType } = useAssetContentType();

const res1 = fetchAssetContentType("https://test.com");
const res2 = fetchAssetContentType("https://test.com");

vi.advanceTimersByTime(3_000);

expect(await res1).toBe("image/png");
expect(await res2).toBe("image/png");

expect(fetch).toHaveBeenCalledOnce();
});
});
29 changes: 29 additions & 0 deletions src/ui/src/composables/useAssetContentType.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
const cacheUrlContentType = new Map<string, Promise<undefined | string>>();

/**
* Do an HTTP `HEAD` call to get the `Content-Type` of an URL. Handle parrallel calls and use a cache mechanism.
*/
export function useAssetContentType() {
function fetchAssetContentType(url: string) {
const cachedValue = cacheUrlContentType.get(url);
if (cachedValue !== undefined) return cachedValue;

// we store the promise instead of the result to handle concurent calls
const promise = fetch(url, { method: "HEAD" })
.then((r) => {
if (!r.ok) return undefined;
return r.headers.get("Content-Type") || undefined;
})
.catch(() => undefined);

cacheUrlContentType.set(url, promise);

return promise;
}

function clearCache() {
cacheUrlContentType.clear();
}

return { fetchAssetContentType, clearCache };
}
22 changes: 22 additions & 0 deletions src/ui/src/utils/url.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import { describe, expect, it } from "vitest";
import { convertAbsolutePathtoFullURL } from "./url";

describe(convertAbsolutePathtoFullURL.name, () => {
it("should convert the URL", () => {
expect(
convertAbsolutePathtoFullURL(
"/assets/image.png",
"http://localhost:3000/",
),
).toBe("http://localhost:3000/assets/image.png");
});

it("should convert the URL with a current path", () => {
expect(
convertAbsolutePathtoFullURL(
"/assets/image.png",
"http://localhost:3000/hello/?foo=bar",
),
).toBe("http://localhost:3000/hello/assets/image.png");
});
});
14 changes: 14 additions & 0 deletions src/ui/src/utils/url.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/**
* Convert absoule URL to full URL in case the application is hosted on a subpath.
*
* ```js
* convertAbsolutePathtoFullURL("/assets/image.png", "http://localhost:3000/hello/?foo=bar")
* // => 'http://localhost:3000/hello/assets/image.png'
* ```
*/
export function convertAbsolutePathtoFullURL(
path: string,
base = window.location.toString(),
) {
return new URL(`.${path}`, base).toString();
}

0 comments on commit 526bdaa

Please sign in to comment.