Skip to content

feat: 선택된 조직명 표시#554

Open
seongminn wants to merge 3 commits into
wip-v2from
org-selector-name
Open

feat: 선택된 조직명 표시#554
seongminn wants to merge 3 commits into
wip-v2from
org-selector-name

Conversation

@seongminn
Copy link
Copy Markdown
Member

✅ 작업 내용

  • 선택된 조직명을 표시했습니다.
  • 조직 선택 시에는 여전히 id만을 관리하고, OrgName 컴포넌트를 추가해서 벼로도 이름을 매핑해주는 로직을 추가했습니다.

📝 참고 자료

  • 작업한 내용에 대한 부연 설명

♾️ 기타

  • 추가로 필요한 작업 내용

@vercel
Copy link
Copy Markdown

vercel Bot commented May 16, 2026

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

Project Deployment Actions Updated (UTC)
spectator Ready Ready Preview, Comment May 16, 2026 2:39am
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
manager Skipped Skipped May 16, 2026 2:39am

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces an OrgName component to display the current organization name in the header and refactors the OrgSwitcher component, including state renaming and memoization of filtered lists. Review feedback highlights a potential regression in the shared Header.Root component where removing a gap class could affect other pages. Additionally, it is recommended to rename selectedOrg to selectedOrgId across the components to more accurately reflect that the state stores a numeric ID, ensuring better code clarity and consistency.

<header className="sticky top-0 z-header h-12 w-full shrink-0 border-b border-neutral-100 bg-white">
<div className="row-between relative h-full w-full px-5">
<div className="center-y gap-4">{left}</div>
<div className="center-y">{left}</div>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

Header.Root 컴포넌트에서 gap-4를 제거하면 이 컴포넌트를 사용하는 다른 페이지들에서 left 슬롯에 여러 요소를 전달할 경우 요소 간의 간격이 사라지는 회귀 버그(Regression)가 발생할 수 있습니다. 공통 컴포넌트의 기본 스타일을 변경하기보다는, Header.Rootgap 관련 prop을 추가하거나 현재처럼 LeftSlot 내부에서 별도의 컨테이너로 간격을 조절하는 방식을 유지하되 기존의 기본값은 보존하는 것이 안전합니다.

Suggested change
<div className="center-y">{left}</div>
<div className="center-y gap-4">{left}</div>

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

기존에 left 속성에 둘 이상의 노드를 전달하는 경우가 없으므로, 회귀 버그 발생하지 않음
그리고 right 속성과 마찬가지로 gap을 추가하지 않고, 이는 사용단에서 관리하는 게 확장성 측면에서 우수함


export const OrgSwitcher = () => {
const [selectedId, setSelectedId] = useState<number | null>(null);
const [selectedOrg, setSelectedOrg] = useState<number | null>(null);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

selectedOrg 상태는 조직 객체가 아닌 ID(number)를 저장하고 있습니다. 변수명을 selectedOrgId로 변경하여 데이터의 성격을 명확히 하고, organizationId와 같은 기존 명명 규칙과 일관성을 유지하는 것이 좋습니다. (해당 파일 내의 모든 selectedOrg 참조를 함께 변경해 주세요.)

Suggested change
const [selectedOrg, setSelectedOrg] = useState<number | null>(null);
const [selectedOrgId, setSelectedOrgId] = useState<number | null>(null);

Comment on lines +84 to +85
selectedOrg: number | null;
handleSelectedOrg: (org: number) => void;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

OrganizationListProps에서도 selectedOrg 대신 selectedOrgId를 사용하고, 핸들러의 인자명도 org 대신 idorgId를 사용하여 타입(number)과 일치시키는 것이 가독성에 도움이 됩니다.

Suggested change
selectedOrg: number | null;
handleSelectedOrg: (org: number) => void;
selectedOrgId: number | null;
handleSelectedOrgId: (id: number) => void;

@seongminn seongminn requested a review from sungwonnoh May 18, 2026 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant