Skip to content
Merged
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
Original file line number Diff line number Diff line change
@@ -1,10 +1,18 @@
"use client";

import { useWindowVirtualizer } from "@tanstack/react-virtual";
import clsx from "clsx";
import { useCallback, useEffect, useRef, useState } from "react";

import type { ListUniversity } from "@/types/university";
import UniversityCard from "../../ui/UniverSityCard";

const UNIVERSITY_CARD_HEIGHT = 91;
const UNIVERSITY_CARD_BOTTOM_PADDING = 10;
const UNIVERSITY_CARD_GAP = 10;
const ESTIMATED_UNIVERSITY_CARD_ROW_HEIGHT = UNIVERSITY_CARD_HEIGHT + UNIVERSITY_CARD_BOTTOM_PADDING;
const INITIAL_VIEWPORT_HEIGHT = 900;

type UniversityCardsProps = {
colleges: ListUniversity[];
style?: React.CSSProperties;
Expand All @@ -14,13 +22,81 @@ type UniversityCardsProps = {
};

const UniversityCards = ({ colleges, style, className, showCapacity = true, linkPrefix }: UniversityCardsProps) => {
const listRef = useRef<HTMLDivElement>(null);
const [scrollMargin, setScrollMargin] = useState(0);

const measureScrollMargin = useCallback(() => {
if (!listRef.current) {
return;
}

setScrollMargin(listRef.current.getBoundingClientRect().top + window.scrollY);
}, []);

useEffect(() => {
measureScrollMargin();

const animationFrameId = window.requestAnimationFrame(measureScrollMargin);
window.addEventListener("resize", measureScrollMargin);

return () => {
window.cancelAnimationFrame(animationFrameId);
window.removeEventListener("resize", measureScrollMargin);
};
}, [measureScrollMargin]);
Comment on lines +36 to +46

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# UniversityCards를 렌더링하는 부모와 위쪽 UI 구성을 확인
rg -nP -C8 '<UniversityCards' apps/university-web/src

Repository: solid-connection/solid-connect-web

Length of output: 4449


scrollMargin이 동적 높이 변화를 놓치지 않도록 ResizeObserver를 함께 소개해 보아요.

반갑게도 현재 코드는 창 크기 조절(resize) 때만 고도를 재고 있어요. 하지만 검색어나 필터 칩이 줄바꿈되거나 비동기 개수 카드가 들어오면 창 크기는 그대로인데 콘텐츠 높이만 춤을 출 수 있죠. 그럴 때 scrollMargin이 구린 값을 잡으면 스크롤 위치가 살짝 어긋나는 상황이 벌어질 수 있어요.

따라서 아래 두 가지 걸음을 함께 걷는 건 어떨까요?

  1. 부모 영역의 호흡을 감지하는 망을 치아요

    • 학술 목록 상단 컨테이너(검색/필터/개수 UI 포함) 에 ResizeObserver 를 얹어두면, 창 크기와 상관없이 높이 변화만으로도 고도 재측정을 자연스럽게 켤 수 있어요.
  2. 정기적인 점검을 유지하면서도 유연하게 대응하아요

    • 기존 resize 이벤트 리스너는 창 크기 변화에 특화된 보조 수단으로 두어, ResizeObserver 가 먼저 변화에 반응하도록 해요.
// 예시 구조
const headerRef = useRef<HTMLElement | null>(null);

useEffect(() => {
  // 기본 측정
  measureScrollMargin();
  const animationFrameId = window.requestAnimationFrame(measureScrollMargin);
  window.addEventListener("resize", measureScrollMargin);

  // 동적 높이 변화 감지
  let resizeObserver: ResizeObserver | null = null;
  if (headerRef.current) {
    resizeObserver = new ResizeObserver(() => {
      measureScrollMargin();
    });
    resizeObserver.observe(headerRef.current);
  }

  return () => {
    window.cancelAnimationFrame(animationFrameId);
    window.removeEventListener("resize", measureScrollMargin);
    resizeObserver?.disconnect();
  };
}, [measureScrollMargin]);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/university-web/src/components/university/UniversityCards/index.tsx`
around lines 36 - 46, `useEffect` in `UniversityCards` only recalculates
`scrollMargin` on `resize`, so it can miss height changes from wrapped
filters/search chips or async card count updates. Update the measurement logic
around `measureScrollMargin` by adding a `ResizeObserver` on the
header/container element that affects the top offset, while keeping the existing
`window.resize` listener and initial `requestAnimationFrame` measurement as
fallback. Make sure the observer is created and cleaned up in the same effect as
`window.addEventListener("resize", ...)` so `scrollMargin` stays accurate when
the content height changes.


const getItemKey = useCallback((index: number) => colleges[index]?.id ?? index, [colleges]);

const virtualizer = useWindowVirtualizer({
count: colleges.length,
estimateSize: () => ESTIMATED_UNIVERSITY_CARD_ROW_HEIGHT,
gap: UNIVERSITY_CARD_GAP,
getItemKey,
overscan: 6,
scrollMargin,
initialRect: {
width: 0,
height: INITIAL_VIEWPORT_HEIGHT,
},
useFlushSync: false,
});
Comment on lines +50 to +62

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor

🧩 Analysis chain

🌐 Web query:

In @tanstack/react-virtualv3 useWindowVirtualizer, does thegap option add spacing in addition to the height returned by measureElement (i.e. element padding plus gap), or does gap replace/include item padding?

💡 Result:

In @tanstack/react-virtual v3, the gap option adds spacing in addition to the size of the items [1][2]. It does not replace or include existing padding within your items; rather, it provides a dedicated mechanism to inject consistent spacing between virtualized items without requiring manual margin or padding adjustments [1][2]. Because the library manages absolute positioning of items, traditional CSS margins on items often do not behave as expected [2][3]. The gap option is designed to handle this by increasing the calculated size/offset of the items so that they remain correctly separated [2]. If you are using dynamic measurements (measureElement), the virtualizer accounts for the measured size of the element and adds the gap value to the space allocated for that item in the virtual list's coordinate system [2][3]. In summary: - The gap option acts as an additive spacing value [2]. - It does not automatically consume or replace the internal padding of your rendered components [1][2]. - It is specifically intended to solve the problem of managing space in an absolutely positioned layout where standard margin-based spacing fails [2].

Citations:


가상 리스트의 간격이 의도치 않게 두 배로 벌어지고 있어요

TanStack Virtual 공식 문서를 확인한 결과, useWindowVirtualizergap 속성은 아이템의 측정된 높이 (measureElement 반환값, 내부 패딩 포함) 에 추가되는 값이라고 합니다. 현재 코드에서는 아이템 래퍼에 pb-2.5(10px) 패딩이 있고, 가상러에도 gap: 10px을 설정하여 실제 간격이 약 20px 로 나타나고 있습니다.

이 문제를 해결하고 시각적 일관성을 확보하기 위해 아래 변경을 제안드립니다.

  1. 간격 중복 제거 (권장)
    가상러 설정의 gap 속성을 제거하고, 간격은 기존 아이템 래퍼의 pb-2.5 패딩 하나로 통일해주세요. UNIVERSITY_CARD_GAP 상수는 더 이상 사용되지 않으므로 함께 정리하면 코드가 깔끔해집니다.

      const virtualizer = useWindowVirtualizer({
        count: colleges.length,
        estimateSize: () => ESTIMATED_UNIVERSITY_CARD_ROW_HEIGHT,
    -    gap: UNIVERSITY_CARD_GAP,
        getItemKey,
        overscan: 6,
        scrollMargin,
  2. 대안: gap 우선 적용
    만약 향후 일정한 간격을 유지하기 위해 gap 속성을 유지하고 싶다면, 아이템 래퍼의 pb-2.5 클래스를 제거하고 estimateSize 계산 시 패딩 값을 제외하여 일치시켜주세요.

  3. 위치 안정성 참고
    스크롤 마진 (scrollMargin) 측정 로직이 상단 콘텐츠 크기 변화 (필터 등) 를 감지하지 못할 경우 아이템 위치가 어긋날 수 있으니, 추후 유동적인 레이아웃이 추가될 시 고려해주시면 좋겠습니다.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const virtualizer = useWindowVirtualizer({
count: colleges.length,
estimateSize: () => ESTIMATED_UNIVERSITY_CARD_ROW_HEIGHT,
gap: UNIVERSITY_CARD_GAP,
getItemKey,
overscan: 6,
scrollMargin,
initialRect: {
width: 0,
height: INITIAL_VIEWPORT_HEIGHT,
},
useFlushSync: false,
});
const virtualizer = useWindowVirtualizer({
count: colleges.length,
estimateSize: () => ESTIMATED_UNIVERSITY_CARD_ROW_HEIGHT,
getItemKey,
overscan: 6,
scrollMargin,
initialRect: {
width: 0,
height: INITIAL_VIEWPORT_HEIGHT,
},
useFlushSync: false,
});
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/university-web/src/components/university/UniversityCards/index.tsx`
around lines 50 - 62, The virtualized list spacing is being applied twice in
UniversityCards because useWindowVirtualizer’s gap is added on top of the item
wrapper padding. Update UniversityCards/index.tsx to use only one source of
spacing: either remove the gap option (and clean up UNIVERSITY_CARD_GAP) while
keeping the pb-2.5 wrapper padding, or remove the wrapper padding and align
estimateSize with the actual item height. Keep getItemKey and scrollMargin
behavior unchanged.


const virtualItems = virtualizer.getVirtualItems();

return (
<div className={clsx("flex flex-col gap-2.5", className)} style={style}>
{colleges.map((college) => (
<div key={college.id} className="pb-2.5">
<UniversityCard university={college} showCapacity={showCapacity} linkPrefix={linkPrefix} />
</div>
))}
<div
ref={listRef}
className={clsx("relative w-full", className)}
role="list"
style={{
...style,
height: `${virtualizer.getTotalSize()}px`,
}}
>
{virtualItems.map((virtualItem) => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Keep detail links in static list HTML

Because this component is rendered on the statically generated /university/[homeUniversity] pages, switching the render loop from colleges.map to virtualItems.map leaves only the initial viewport of UniversityCard anchors in the pre-rendered/no-JS HTML; the rest of the detail links are created only after client JS runs and the user scrolls. Since sitemap() currently emits only /university, /university/search, and /university/${slug} entries (apps/university-web/src/app/sitemap.ts:18-37), crawlers or no-JS users lose discovery of most detail pages generated by generateStaticParams, so please keep a server/no-JS fallback with all links or add an equivalent discoverable source.

Useful? React with 👍 / 👎.

const college = colleges[virtualItem.index];

if (!college) {
return null;
}

return (
<div
key={virtualItem.key}
ref={virtualizer.measureElement}
className="absolute left-0 top-0 w-full pb-2.5"
data-index={virtualItem.index}
role="listitem"
aria-posinset={virtualItem.index + 1}
aria-setsize={colleges.length}
style={{
transform: `translateY(${virtualItem.start - virtualizer.options.scrollMargin}px)`,
}}
>
<UniversityCard university={college} showCapacity={showCapacity} linkPrefix={linkPrefix} />
</div>
);
})}
</div>
);
};
Expand Down
Loading