feat: 대학 세부 페이지 디자인 변경 #188

wibaek · 2025-06-24T08:16:20Z

실수로 제 레포지토리에 브랜치를 안파고 하던대로 solid-connection 레포지토리에 브랜치를 파서 올렸네요😅, 다음부터 수정하도록 하겠습니다~

작업 내용

피그마 SLCN Design System(App) -> Design -> 학교 정보 상세페이지에 있는 신규 상세페이지 디자인으로 변경했습니다. 생생한 후기와 지역정보는 아직 백엔드 api가 없어서 추가하지 않았습니다. 멘토링 기능들 추가 후기 백엔드에 말씀드려서 추가 예정입니다.

페이지는 섹션별로 컴포넌트로 분리했습니다. 예) 어학성적, 기타 정보, 전공상세, 영어강의 리스트

이때 섹션별로 제목이 자주 반복되어 HeaderZone으로 분리했습니다. (위 사진 참고)

LanguageSection에서는 데이터가 TOEFL_IBT와 같은 방식으로 들어오고, 로고명도 맵핑해주어야 해서, 이를 일괄적으로 처리할 languageUtils를 추가했습니다.

MajorSection, EnglishSection 는 구조가 사실상 같아 컴포넌트화 할까 고민했지만, 2번 까지는 중복을 유지해도 괜찮다고 생각하여 그대로 두었습니다.

InfoSection의 최저 이수학기, 파견 가능학기의 value가 디자인적으로 문제가 발생할 수 있습니다. 이건 백엔드쪽 데이터 전처리가 필요해보입니다.

특이 사항

TODO: 시험 icon 없을시 default.png 변경 필요, SubTitleSection totalDispatchCount 추가시 연동, 나라에 국기 추가

리뷰 요구사항 (선택)

과감하게 리뷰해주셔도 좋아요!

지도 관련해서 많이 다루어보시는 걸로 아는데, GoogleEmbedMap 관련해서도 피드백이 있으시면 부탁드립니다!

…, LanguageSection 추가

manNomi

제가 합류한 지 얼마 되지 않아, 코드 스타일이나 팀의 컨벤션에 아직 익숙하지 않은 부분이 있습니다.
혹시 제가 잘못 이해하고 드린 피드백이 있다면 편하게 말씀해주시면 감사하겠습니다!

리뷰를 드리면서도 혹시 기존 흐름이나 팀 기준과 다르게 보았을까 조심스러운 마음이 있어 이렇게 함께 남깁니다.
부족한 부분은 언제든 과감하게 피드백 주셔도 괜찮습니다 :)

manNomi · 2025-06-25T13:17:15Z

src/app/university/[id]/HeaderZone.tsx

@@ -0,0 +1,10 @@
+const HeaderZone = ({ title, children }: { title: string; children?: React.ReactNode }) => {


코드리뷰에서 작성해주셨던 부분으로 분리해서 여러군데 사용할 수 있어서 좋은것 같습니다 !

HeaderZone을 확인하기 위해서 컴포넌트를 여러개를 봐야하는데 컴포넌트의 코드가 디자인 요소인데 한줄이라 합쳐도 무리 없지 않을 수 도 있겠다고 생각이듭니다 !
아니면 컴포넌트의 디자인 컴포넌트를 만들어서 간단한 요소들을 모두 분리하는것 도 방법은 어떨까요 ?

최대한 공통되는걸 분리해봐야지 했는데, 공통요소가 그렇게 크지 않아 애매해진것 같네요. 😅

컴포넌트의 코드가 디자인 요소인데 한줄이라 합쳐도 무리 없지 않을까요
-> 나중에 변경이 일어나면 다 바꿔줘야하지만 변경일어날 확률이 작고, 그렇게 귀찮은 작업은 아닐것 같아 삭제해도 무방할 것 같네요
-> 아니면 HeaderZone이 모호해서 차라리 Header만 분리해서 Header로 바꾸는것도 좋을 것 같다고 느껴지네요
-> 결론적으로 HeaderZone은 삭제하고 그대로 쓰는게 좋을 것 같습니다!

컴포넌트의 디자인 컴포넌트를 만들어서 간단한 요소들을 모두 분리하는것 도 방법은 어떨까요 ?
-> 추가적으로 분리하자면
와 EnglishSection/MajorSection 정도인 것 같은데 분리할 요소가 크게 없는 것 같긴 합니다.

그래서 최종적으로 1번 안으로 가는게 좋을 것 같네요! 어떻게 생각하시나요?

너무 좋은 의견인것 같습니다 !!

manNomi · 2025-06-25T13:23:40Z

src/app/university/[id]/InfoSection.tsx

+
+import { useState } from "react";
+
+const InfoSection = ({


뷰랑 로직이 분리가 잘되어서 깔끔한것 같습니다

manNomi · 2025-06-25T13:44:36Z

src/app/university/[id]/LanguageSection.tsx

+              <Language
+                key={`${req.languageTestType}-${idx}`}
+                name={getDisplayName(req.languageTestType)}
+                logoUrl={getLanguageLogo(req.languageTestType)}


public 이미지를 map을 통해서 관리하니 개발할때 매우 편할것 같습니다 !!

manNomi · 2025-06-25T13:46:08Z

src/app/university/[id]/LanguageSection.tsx

+            {languageRequirements.map((req, idx) => (
+              <Language
+                key={`${req.languageTestType}-${idx}`}
+                name={getDisplayName(req.languageTestType)}


getDisplayName의 함수 명으로는 무엇을 보여주는지 이해하기 조금은 힘들수도 있을것 같습니다
함수명을 조금더 직관적으로 바꿔보는것 도 좋을것 같습니다

getDisplayName() -> formatLanguageTestName()
getLanguageLogo() -> getLanguageTestLogo() 로 변경했습니다!

manNomi · 2025-06-25T13:55:46Z

src/app/university/[id]/LanguageSection.tsx

+  );
+};
+
+const Language = ({ name, logoUrl, score }: { name: string; logoUrl: string; score: string }) => {


컴포넌트를 분리하신 이유가 있으신가요?
현재 구조에서는 해당 컴포넌트가 다른 곳에서 재사용되지 않고, 로직도 단순해서 한 파일 내에 두는 게 오히려 응집도나 가독성 면에서 더 나을 수 있을 것 같습니다.

개인적으로는 한 파일 내에서 논리적으로 묶인 UI가 함께 있는 구조가 더 유지보수하기 좋다고 생각하는데, 혹시 분리하신 특별한 이유가 있다면 공유 부탁드립니다!

코드의 분리로 가독성이 더 좋은것 같기도해서 의견이 궁금합니다 !!

Language를 분리한 이유를 물어보신 것 맞으시죠?

현재도 한 파일내에 위치하는데, 한 파일내에 두는게 더 응집도가 높아진다는 것에 대해 다시 설명해주실 수 있을까요..?

컴포넌트 분리의 이유는 재사용되지는 않지만 Language는 깊이가 꽤 있어서 분리하는게 더 이해하기 좋을거라 생각했습니다.

제가 말씀드린 응집도는 기능적으로 밀접하게 연결된 코드들이 하나의 파일 안에 함께 있을 때, 전체 흐름을 파악하거나 수정 포인트를 빠르게 예측할 수 있다는 의미에서 사용한 표현이었습니다.
예를 들어 Language 컴포넌트가 해당 페이지에서만 사용되고 관련 상태나 로직과도 강하게 연결되어 있다면 같은 파일 내에 두는 편이 유지보수에 더 유리하다고 느꼈습니다.

한 파일에 여러 컴포넌트가 선언되어 있는 것이 어색하게 느껴져 이와 관련해서는
팀 내에서 다음과 같은 기준을 논의해보고 정해보는 것도 좋을 것 같습니다:
1. 1파일 1컴포넌트를 원칙으로 가져가기
2. 재사용되지 않지만 depth가 깊거나 로직이 무거운 경우에는 분리를 허용
• 2-1. 우선은 동일 파일 내부에서 분리된 형태로 작성하거나
• 2-2. 필요시 /components/[해당 페이지명]/ 아래에 페이지 종속 컴포넌트로 위치시킨다

UI가 깊거나 복잡할 경우에는 분리를 통해 가독성을 높이는 것도 충분히 유효한 선택이라고 생각합니다.
상황에 따라 유연하게 판단하면서, 앞으로도 이런 기준들을 조금씩 정립해 나가면 좋을 것 같습니다! 의견 나눠주셔서 감사합니다 😊

manNomi · 2025-06-25T14:03:07Z

src/app/university/[id]/EnglishSection.tsx

+      <div className="my-7 px-3">
+        <HeaderZone title="영어강의 리스트">
+          <span className="break-words text-sm font-medium leading-normal text-k-600">
+            <ReactLinkify>{englishDetail}</ReactLinkify>


UX상 url의 커서만 바뀌는데 실제 사용자가 클릭이 되는지 모를 수도 있을것 같습니다
간단한 썸네일을 보여주거나 호버시 파랑색으로 색상을 바꿔주는등의 방향은 어떤가요 ?

좋은 것 같습니다!

해당 부분은 다른 PR로 뺴서 작업하겠습니다.

왜냐하면 react-linkify -> linkify-react 라이브러리 변경이 필요할 것 같아서 PR의 크기가 커질 것 같아서 그렇습니다!

manNomi · 2025-06-25T14:17:47Z

src/app/university/[id]/MapSection.tsx

+      <div className="h-1 bg-k-50" />
+      <div className="my-7">
+        <HeaderZone title="파견학교 위치">
+          <GoogleEmbedMap width="100%" height="300px" name={universityEnglishName} style={{ border: "0" }} />


구글 임베드맵을 실제로 사용할 일이 없어서 활용해보진 못했었는데,
해당 코드를 보니 서버 컴포넌트에서 사용할 수 있다는 점에서 매우 좋은 선택이라는 생각이 들었습니다.
특히 SSR이나 SSG 환경에서는 iframe 기반 임베드맵만이 선택지라는 점에서 구조적으로도 잘 설계된 것 같습니다.

다만, 구글지도 과금 정책 기준으로 보면 Embed API는 Dynamic Maps API보다 무료 호출량이 절반 정도라
향후 대규모 유저가 많아질 경우 과금 리스크도 고려해보면 좋을 것 같습니다.

그리고 아래 스크린샷처럼 현재 임베드맵의 좌측 상단에 나오는 정보 카드 때문에
사용자가 지도의 지리적 위치를 한눈에 보기 어렵거나 시야를 가릴 수 있는 부분이 있는 것 같습니다.

이 부분은 다음과 같은 방법으로 개선해볼 수 있을 것 같은데, 혹시 어떻게 생각하시나요?
• view 모드로 변경해 정보 카드 제거 (/embed/v1/view)
• 또는 줌 레벨을 조정해 전체 맵을 좀 더 넓게 보여주기

좋습니다! 이것도 별도 이슈로 빼서 두가지 모두 시도해보고 PR 올리겠습니다. 피드벡 감사합니다☺️

가격도 확인해봤는데 가격 정책은 Embed가 무료인 것 같습니다!

제가 잘못알고 있었던 부분이었는데 덕분에 저도 지식을 얻어가는 것 같습니다

manNomi · 2025-06-25T14:19:00Z

src/app/university/[id]/SubTitleSection.tsx

@@ -0,0 +1,17 @@
+interface SubTitleSectionProps {


특정 컴포넌트는 타입을 별도 선언하고, 다른 컴포넌트는 인라인으로 작성하신 이유가 있을까요?

모두 interface로 정의하는게 컨벤션상 맞습니다. 컨벤션 문서에 추가하고, 수정하겠습니다. 감사합니다!

다음 컨벤션은 괜찮으신가요?

React 컴포넌트 props는 모두 interface로 타입을 정의한다. 이때 타입의 이름은 끝에 Props를 추가한 이름으로 한다. 예시) ReactComponentProps

이러면

일관성을 확보할 수 있음

줄로 구분되어 보기 좋고, git 관리가 편해짐

type이 아닌 interface 사용의 이유 -> type/interface는 프로젝트에서 통일하는게 좋은데, 해당 프로젝트의 다른 부분에는 interface를 사용하고 있음

의 장점이 있다고 생각합니다!

너무 좋습니다 제가 팀에 합류한 직후이기때문에 컨벤션을 맞추는 과정이라고 생각합니다 !!!

manNomi · 2025-06-25T14:21:30Z

src/app/university/[id]/UniversityDetail.tsx

+
+const UniversityDetail = ({ university }: { university: University }) => {
+  return (
+    <>


일부 컴포넌트는 <></> 프래그먼트로 감싸고, 일부는
등 실제 DOM 요소로 감싸고 있어서, 개발자 도구(Elements 탭)에서 구조를 추적할 때 조금 헷갈릴 수 있는 부분이 있는 것 같습니다
혹시 해당 위치에 프래그먼트를 사용하신 특별한 이유나 기준이 있으실까요?

div가 필요한곳이 아니라면 최대한 사용을 지양하는게 좋다고 생각했습니다. 왜냐하면 깊이가 깊어지기 떄문인데요, 생각해보니 div 사용을 안했을 때 깊이가 줄어드는것보다 div가 없어서 헷갈리는게 더 크다고 느껴지네요. div로 감싸도록 하겠습니다!

말씀해주신 depth가 깊어질 경우 테스트가 어려워질 수 있다는 점도 충분히 타당하다고 생각합니다.
결국 중요한 건 일관된 기준이라고 생각되기 때문에, 해당 부분은 컨벤션으로 정해서 맞춰 나가면 큰 무리는 없을 것 같습니다.
depth가 깊어지면 테스트 및 CSS 조작 측면에서도 불리할 수 있기에, 프래그먼트(<></>)로 감싸는 방식도 충분히 유효한 선택이라고 생각합니다
각 컴포넌트를 하나의 div로 감싸는 방식으로 통일한다면, 테스트 시에도 해당 div를 기준으로 컴포넌트를 구분할 수 있고, 각 파일이 자신의 UI 책임만 지는 구조로 유지할 수 있어서 유지보수 측면에서도 괜찮은 방향이라고 생각합니다.

따라서 일관된 기준을 하나 정해서 하는것은 어떨지 여쭙고 싶습니다
좋은 논의 감사드리고 앞으로도 이런 기준들 하나씩 정리해보면 좋겠습니다! 🙌

manNomi · 2025-06-25T14:21:55Z

src/app/university/[id]/UniversityDetail.tsx

+
+export default UniversityDetail;
+
+const UniversityImage = ({ imageUrl }: { imageUrl: string }) => {


해당 컴포넌트를 별도로 분리하신 이유가 있을까요?

현재 구조에서는 이 컴포넌트가 한 곳에서만 사용되고, 복잡도도 높지 않아서
오히려 한 파일 내에 함께 두는 편이 응집도나 가독성 면에서 더 좋을 수 있겠다는 생각이 들었습니다.

물론, 이전처럼 컴포넌트를 분리하면 전반적인 흐름을 명확히 나누는 데 도움이 될 수 있다는 점도 공감됩니다.
다만 개인적으로는 불필요한 분리를 줄이는 쪽이 유지보수나 코드 파악 측면에서 더 유리하다고 생각해서 여쭤보게 되었습니다.

혹시 분리하신 데에 특별한 기준이나 고려하신 부분이 있으셨다면 공유해주시면 감사하겠습니다!!

이름을 지어주는게 가독성이 더 높을 수 있다고 생각했습니다. 확인해보니 UniversityDetail는 굳이 분리하지 않아도 충분히 가독성이 있었을 것 같네요. 이 부분은 합치는 방향으로 작업하겠습니다.

getDisplayName() -> formatLanguageTestName() getLanguageLogo() -> getLanguageTestLogo()

wibaek added 9 commits June 24, 2025 00:33

feat: 신규 학교 세부 페이지 UniversityDetail 추가, TitleSection, SubTitleSection…

4622a45

…, LanguageSection 추가

feat: 어학세부요건 추가

d92a2b1

feat: MajorSection, EnglishSection 추가

a0b886b

feat: MapSection 추가

59dc2d6

feat: InfoSection 추가

9837c8a

refactor: rename collegeData -> universityData

2ae02bc

feat: 실제 데이터 연결

6349210

remove: 과거 디자인, 미사용 코드 제거

3340368

feat: languageUtils 추가, LanguageSection 실제 데이터 연결

c1268ee

wibaek requested a review from manNomi June 24, 2025 08:16

wibaek self-assigned this Jun 24, 2025

wibaek added the feature New feature or request label Jun 24, 2025

wibaek changed the title ~~Feature/univ detail~~ feat: 대학 세부 페이지 디자인 변경 Jun 24, 2025

manNomi reviewed Jun 25, 2025

View reviewed changes

wibaek added 2 commits June 26, 2025 14:21

refactor: 함수명 더 직관적으로 변경

3938fe5

getDisplayName() -> formatLanguageTestName() getLanguageLogo() -> getLanguageTestLogo()

refactor: 프래그먼트 <></> -> 실제 DOM요소로 변경

0bedbdb

wibaek mentioned this pull request Jun 26, 2025

ReactLinkify를 개선한다 #191

Closed

3 tasks

wibaek requested a review from manNomi June 26, 2025 06:03

wibaek mentioned this pull request Jun 26, 2025

학교 상세페이지 지도를 개선한다 #190

Open

3 tasks

wibaek added 5 commits June 26, 2025 15:18

refactor: prop types interface 분리

8af454a

refactor: HeaderZone 삭제

01fad0d

refactor: revert 프래그먼트 <></> -> 실제 DOM요소로 변경

2064de4

refactor: 미사용 코드 삭제

aef5814

refactor: UniversityDetail 과도한 컴포넌트 분리 해제

6ab5378

wibaek merged commit 9d737ec into main Jun 30, 2025

manNomi deleted the feature/univ-detail branch December 10, 2025 03:57

		@@ -0,0 +1,10 @@
		const HeaderZone = ({ title, children }: { title: string; children?: React.ReactNode }) => {


		export default UniversityDetail;

		const UniversityImage = ({ imageUrl }: { imageUrl: string }) => {

feat: 대학 세부 페이지 디자인 변경 #188

feat: 대학 세부 페이지 디자인 변경 #188

Uh oh!

Conversation

wibaek commented Jun 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

관련 이슈

작업 내용

특이 사항

리뷰 요구사항 (선택)

Uh oh!

manNomi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wibaek Jun 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wibaek Jun 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

wibaek commented Jun 24, 2025 •

edited

Loading

wibaek Jun 26, 2025 •

edited

Loading

wibaek Jun 26, 2025 •

edited

Loading