Skip to content

[CHORE] 기수별 멤버 아코디언에 높이 정책 추가#565

Open
ej070961 wants to merge 1 commit intodevelopfrom
chore/member-accordion
Open

[CHORE] 기수별 멤버 아코디언에 높이 정책 추가#565
ej070961 wants to merge 1 commit intodevelopfrom
chore/member-accordion

Conversation

@ej070961
Copy link
Contributor

@ej070961 ej070961 commented Mar 6, 2026

✅ 체크리스트

  • 코드에 any가 사용되지 않았습니다
  • 스토리북에 추가했습니다 (해당 시)
  • 이슈와 커밋이 명확히 연결되어 있습니다

🔗 관련 이슈

📌 작업 목적

  • 기수별 멤버 아코디언에 높이 정책을 동적으로 주입받도록 변경

🔨 주요 작업 내용

  • MemberGenerationAccordion에 하드코딩되어 있던 maxHeight 제거, 최대 높이를 동적으로 주입받도록 변경
  • 기수별 멤버 페이지 사이즈 기능 요구사항에 맞게 20명으로 변경

⚠️ 기존 문제 (선택)

  • MemberGenerationAccordion에 maxHeight이 하드코딩 되어 있어 화면 전체를 차지해야하는 그룹 관리 화면에서도 maxHeight이 적용되는 이슈 발생
image

☑️ 해결 방법 (선택)

  • MemberGenerationAccordion에 하드코딩되어 있던 maxHeight 제거, 최대 높이를 동적으로 주입받도록 변경

Summary by CodeRabbit

릴리스 노트

  • 개선 사항
    • 세대별 멤버 목록 페이지당 로드 항목 수를 5개에서 20개로 증가하여 페이지네이션 효율성 개선
    • 멤버 생성 아코디언 컴포넌트의 높이 제어 유연성 향상

@ej070961 ej070961 self-assigned this Mar 6, 2026
@vercel
Copy link

vercel bot commented Mar 6, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
surf-admin Ready Ready Preview, Comment Mar 6, 2026 7:40am
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
surf-web Skipped Skipped Mar 6, 2026 7:40am

@coderabbitai
Copy link

coderabbitai bot commented Mar 6, 2026

📝 Walkthrough

워크스루

멤버 목록 아코디언 컴포넌트의 높이를 동적으로 조정할 수 있도록 변경하는 리팩토링입니다. 고정된 36vh 최대 높이 클래스를 제거하고 contentMaxHeight 옵션 프로퍼티를 추가하여 부모 컴포넌트에서 높이 정책을 주입받도록 구조화했습니다. 동시에 무한 스크롤 페이지 크기를 5에서 20으로 증가시켜 한 번에 요청하는 데이터 양을 늘렸습니다.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes


코드 품질 검토

✅ 긍정적인 측면

  1. 확장성 개선: 아코디언 높이를 동적으로 주입받도록 변경하여 다양한 레이아웃 요구사항에 대응할 수 있게 되었습니다.

  2. 관심사 분리: 높이 정책(ACCORDIAN_MAX_HEIGHT)을 사용처인 MemberGenerationAccordionList에서 관리하도록 배치하여 책임이 명확합니다.

  3. 하위 호환성: contentMaxHeight를 선택적 프로퍼티로 정의하여 기존 사용처에 영향을 주지 않습니다.


⚠️ 개선 권장사항

1. 타입 안정성 강화

// 현재 (위험)
contentMaxHeight?: string

// 권장
contentMaxHeight?: `${number}${'vh' | 'px' | '%' | 'rem'}` | 'auto'

이유: 부모에서 잘못된 CSS 값(예: "hello")을 전달할 수 있습니다. 유니온 타입으로 유효한 CSS 단위로 제한하세요.

2. 상수 관리 개선

ACCORDIAN_MAX_HEIGHT 정의 위치를 고려하세요:

  • 현재: 컴포넌트 파일 내부 (지역 상수)
  • 권장: 공유 상수 파일(apps/admin/src/shared/ui/constants/accordion.constants.ts) 또는 컴포넌트 라이브러리 설정 파일

이유: 추후 다른 컴포넌트에서도 동일한 높이값이 필요할 때 재사용 가능하고, 디자인 토큰 관리가 용이합니다.

3. 접근성 검증

인라인 스타일로 maxHeight 설정 시:

// 스크린 리더 사용자를 위한 고려
// overflow-y-auto 상태에서 포커스 관리 확인
<div style={{ maxHeight: contentMaxHeight }} className="overflow-y-auto">
  {/* 스크롤 내용물에 aria-label이나 role 설정 필요? */}
</div>

확인 사항:

  • 높이 제한으로 인한 콘텐츠 숨김이 스크린 리더에 영향을 주지 않는지 확인
  • 포커스 가능한 요소가 스크롤 영역 내에 있을 때 포커스 트랩(focus trap) 관리

4. 페이지 크기 변경의 영향도 검토

// useMembersByGenerationInfiniteQuery.ts
const PAGE_SIZE = 20 // 5에서 변경

// 확인 필요:
// 1. 초기 로딩 성능 - API 응답 시간 증가?
// 2. 메모리 캐싱 - 동시에 여러 세대(generation)에서 활성화될 경우 메모리 압박?
// 3. 무한 스크롤 트리거 시점 - 4배 데이터로 인한 UX 변화 감지?

권장: 성능 모니터링과 함께 배포하여 실제 영향을 측정하세요.

5. 테스트 추가

현재 변경사항에 대한 테스트가 없어 보입니다:

// 테스트 예시
describe('MemberGenerationAccordion', () => {
  it('contentMaxHeight prop이 전달될 때 style.maxHeight에 반영되어야 함', () => {
    // 32vh 전달 시 스타일에 적용되는지 확인
  });

  it('contentMaxHeight가 없을 때 기본값이 36vh여야 함', () => {
    // 폴백 동작 확인
  });
});

6. HTML/CSS 검증

// 인라인 스타일 적용 시 우선순위 확인
<div 
  style={{ maxHeight: contentMaxHeight }} 
  className="overflow-y-auto max-h-[36vh]" // ❌ max-h-[36vh]와 충돌 가능
>

확인: Tailwind 클래스와 인라인 스타일이 겹치지 않는지 검증하세요.


📋 Clean Architecture 관점

준수 사항:

  • 프레젠테이션 로직(높이 값)이 프리젠테이션 레이어에 남음

⚠️ 개선 제안:

  • 상수 관리 전략 수립: 매직 값('36vh')을 설정 계층으로 추상화
  • 컴포넌트 고도화: 높이 외 다른 스타일 정책도 동적으로 주입받을 구조로 확장성 고려
🚥 Pre-merge checks | ✅ 6
✅ Passed checks (6 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed PR 제목은 주요 변경사항인 아코디언 높이 정책 추가를 명확하게 요약하고 있으며, 실제 코드 변경과 일치합니다.
Linked Issues check ✅ Passed PR이 연결된 이슈 #564의 요구사항을 완전히 충족합니다: 아코디언 높이를 동적으로 주입받도록 변경하고, 페이지 크기도 20으로 조정했습니다.
Out of Scope Changes check ✅ Passed 모든 변경사항이 아코디언 높이 정책 추가라는 명확한 범위 내에 있으며, 범위를 벗어난 변경은 없습니다.
Description check ✅ Passed PR 설명은 변경 목적, 동기, 구체적인 변경사항을 명확하게 기술하고 있습니다.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chore/member-accordion

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@apps/admin/src/features/member-by-generation/ui/MemberGenerationAccordion.tsx`:
- Around line 18-27: MemberGenerationAccordion's optional prop contentMaxHeight
has no default, causing maxHeight to be undefined and allowing uncontrolled
expansion; restore the prior default by treating contentMaxHeight as
contentMaxHeight ?? '36vh' inside MemberGenerationAccordion (and interpret a
literal "none" as the opt‑in to disable the cap) so the rendered style uses the
default 36vh unless contentMaxHeight === 'none'; update the call sites (e.g.,
MemberSearchAccordionList.tsx) and the test MemberGenerationAccordion.test.tsx
as needed to rely on the component default rather than passing the prop
everywhere.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 5fd59f45-4ad0-47ee-ba44-f371873df9c7

📥 Commits

Reviewing files that changed from the base of the PR and between f9fc45c and 00945e6.

📒 Files selected for processing (3)
  • apps/admin/src/features/member-by-generation/model/useMembersByGenerationInfiniteQuery.ts
  • apps/admin/src/features/member-by-generation/ui/MemberGenerationAccordion.tsx
  • apps/admin/src/widgets/member-directory/ui/MemberGenerationAccordionList.tsx

Comment on lines +18 to +27
contentMaxHeight?: string;
};

export const MemberGenerationAccordion = ({ generation, label, keyword, renderItem }: Props) => {
export const MemberGenerationAccordion = ({
generation,
label,
keyword,
renderItem,
contentMaxHeight,
}: Props) => {
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for MemberGenerationAccordion usages in the codebase
rg "MemberGenerationAccordion" -t tsx -t ts -A 3 -B 1

Repository: Tave-Makers/SURF-FE

Length of output: 90


🏁 Script executed:

#!/bin/bash
# Run the Python script from the review to check for usages without contentMaxHeight
python3 - <<'PY'
from pathlib import Path
import re

pattern = re.compile(r"<MemberGenerationAccordion\b(?P<body>.*?)(?:/>|</MemberGenerationAccordion>)", re.S)
found = False

for path in Path(".").rglob("*.tsx"):
    if "node_modules" in path.parts or ".next" in path.parts:
        continue
    try:
        text = path.read_text(encoding="utf-8")
    except:
        continue
    for match in pattern.finditer(text):
        block = match.group(0)
        if "contentMaxHeight=" not in block:
            found = True
            line = text.count("\n", 0, match.start()) + 1
            print(f"{path}:{line}")
            print(block[:300].strip())
            print("-" * 80)

if not found:
    print("All MemberGenerationAccordion call sites explicitly pass contentMaxHeight.")
PY

Repository: Tave-Makers/SURF-FE

Length of output: 773


🏁 Script executed:

#!/bin/bash
# Check the component implementation to see the actual style application
cat -n apps/admin/src/features/member-by-generation/ui/MemberGenerationAccordion.tsx | head -80

Repository: Tave-Makers/SURF-FE

Length of output: 2834


🏁 Script executed:

cat -n apps/admin/src/widgets/group-management/ui/MemberSearchAccordionList.tsx | head -50

Repository: Tave-Makers/SURF-FE

Length of output: 1843


🏁 Script executed:

# Also check if there's a git history or any recent changes to understand the original default
git log --oneline -20 -- apps/admin/src/features/member-by-generation/ui/MemberGenerationAccordion.tsx 2>/dev/null || echo "Git history not available"

Repository: Tave-Makers/SURF-FE

Length of output: 101


기본 높이 정책이 사라져 실제 코드에 회귀가 발생합니다.

검증 결과, 제시하신 우려가 정당합니다. apps/admin/src/widgets/group-management/ui/MemberSearchAccordionList.tsx:27실제 프로덕션 코드에서 contentMaxHeight prop을 전달하지 않고 있습니다. 이로 인해 이 컴포넌트의 아코디언 높이 제약이 사라지고 무제한 확장됩니다(이전 기본값 36vh → 현재 undefined).

또한 MemberGenerationAccordion.test.tsx:12의 테스트 코드도 동일한 문제가 있습니다.

원인: contentMaxHeight가 optional prop이면서 컴포넌트 내 기본값이 없어서, prop을 전달하지 않으면 style={{ maxHeight: undefined }}가 되어 높이 제약이 적용되지 않습니다.

권장 해결책:

  1. 방법 A (권장): 컴포넌트 내 기본값 복원 → contentMaxHeight ?? '36vh'로 처리하되, full-height가 필요한 경우만 contentMaxHeight="none"으로 opt-in
  2. 방법 B: 기존 모든 호출부에 contentMaxHeight="36vh" 명시 추가 (더 번거로움)

방법 A가 기존 화면 회귀를 막으면서도 새로운 요구사항을 충족하는 안전한 선택입니다.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/admin/src/features/member-by-generation/ui/MemberGenerationAccordion.tsx`
around lines 18 - 27, MemberGenerationAccordion's optional prop contentMaxHeight
has no default, causing maxHeight to be undefined and allowing uncontrolled
expansion; restore the prior default by treating contentMaxHeight as
contentMaxHeight ?? '36vh' inside MemberGenerationAccordion (and interpret a
literal "none" as the opt‑in to disable the cap) so the rendered style uses the
default 36vh unless contentMaxHeight === 'none'; update the call sites (e.g.,
MemberSearchAccordionList.tsx) and the test MemberGenerationAccordion.test.tsx
as needed to rely on the component default rather than passing the prop
everywhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Chore] 멤버 목록 아코디언 FullHeight 가능하도록 변경

1 participant