Skip to content
Open
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
2 changes: 2 additions & 0 deletions src/components/ImageWithError.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,15 @@ import { createSignal, JSXElement, Show } from "solid-js"
export const ImageWithError = <C extends ElementType = "img">(
props: ImageProps<C> & {
fallbackErr?: JSXElement
onLoad?: () => void
},
) => {
const [err, setErr] = createSignal(false)
return (
<Show when={!err()} fallback={props.fallbackErr}>
<Image
{...props}
onLoad={props.onLoad}
Copy link

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The onLoad prop is redundantly specified both in the spread operator and explicitly. Since props are spread with {...props}, the explicit onLoad={props.onLoad} is unnecessary. Remove the explicit onLoad line or restructure to avoid redundancy.

Suggested change
onLoad={props.onLoad}

Copilot uses AI. Check for mistakes.
onError={() => {
setErr(true)
}}
Expand Down
37 changes: 35 additions & 2 deletions src/pages/home/folder/GridItem.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Center, VStack, Icon, Text } from "@hope-ui/solid"
import { Motion } from "solid-motionone"
import { useContextMenu } from "solid-contextmenu"
import { batch, Show } from "solid-js"
import { batch, Show, createSignal, onMount, onCleanup } from "solid-js"
import { CenterLoading, LinkWithPush, ImageWithError } from "~/components"
import { usePath, useRouter, useUtil } from "~/hooks"
import { checkboxOpen, getMainColor, local, selectIndex } from "~/store"
Expand All @@ -27,8 +27,37 @@ export const GridItem = (props: { obj: StoreObj; index: number }) => {
const { pushHref, to } = useRouter()
const { openWithDoubleClick, toggleWithClick, restoreSelectionCache } =
useSelectWithMouse()

const [isVisible, setIsVisible] = createSignal(false)
Copy link

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The isVisible signal is set but never used in the component. This adds unnecessary state management overhead. Consider removing this signal if it's not needed for the lazy loading implementation.

Copilot uses AI. Check for mistakes.
const [loaded, setLoaded] = createSignal(false)
const [canLoad, setCanLoad] = createSignal(false)
let ref: HTMLDivElement | undefined
let loadTimeout: number | undefined
Copy link

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The loadTimeout variable is typed as 'number | undefined', but in some TypeScript configurations (particularly with Node.js types), window.setTimeout returns a NodeJS.Timeout object rather than a number. This can cause type errors. Use 'ReturnType' or cast the result to ensure type compatibility across different environments.

Suggested change
let loadTimeout: number | undefined
let loadTimeout: ReturnType<typeof setTimeout> | undefined

Copilot uses AI. Check for mistakes.

onMount(() => {
if (ref) {
Copy link

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This use of variable 'ref' always evaluates to false.

Copilot uses AI. Check for mistakes.
const observer = new IntersectionObserver(
([entry]) => {
if (entry.isIntersecting) {
Copy link

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The timeout is not cleared when a new intersection event occurs while a previous timeout is still pending. If the element rapidly enters and exits the viewport, multiple setTimeout callbacks could be queued, potentially causing canLoad to be set to true even after the element has left the viewport. Clear the existing timeout before setting a new one.

Suggested change
if (entry.isIntersecting) {
if (entry.isIntersecting) {
if (loadTimeout) clearTimeout(loadTimeout)

Copilot uses AI. Check for mistakes.
loadTimeout = setTimeout(() => setCanLoad(true), 500)
} else {
if (loadTimeout) clearTimeout(loadTimeout)
setCanLoad(false)
}
setIsVisible(entry.isIntersecting)
Copy link

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The isVisible signal is updated but never used anywhere in the component logic or rendering. This adds unnecessary overhead. Either remove this line or utilize the signal if it's intended for future use.

Copilot uses AI. Check for mistakes.
},
{ rootMargin: "50px" },
)
observer.observe(ref)
onCleanup(() => {
observer.disconnect()
if (loadTimeout) clearTimeout(loadTimeout)
})
}
})
return (
<Motion.div
ref={ref}
initial={{ opacity: 0, scale: 0.9 }}
animate={{ opacity: 1, scale: 1 }}
transition={{ duration: 0.2 }}
Expand Down Expand Up @@ -115,7 +144,10 @@ export const GridItem = (props: { obj: StoreObj; index: number }) => {
}}
/>
</Show>
<Show when={props.obj.thumb} fallback={objIcon}>
<Show
when={props.obj.thumb && (canLoad() || loaded())}
fallback={objIcon}
>
<ImageWithError
maxH="$full"
maxW="$full"
Expand All @@ -125,6 +157,7 @@ export const GridItem = (props: { obj: StoreObj; index: number }) => {
fallbackErr={objIcon}
src={props.obj.thumb}
loading="lazy"
onLoad={() => setLoaded(true)}
/>
</Show>
</Center>
Expand Down