Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Refactor delete/#55 #57

Merged
merged 22 commits into from
Apr 1, 2024
Merged

Refactor delete/#55 #57

merged 22 commits into from
Apr 1, 2024

Conversation

gahyuun
Copy link
Member

@gahyuun gahyuun commented Mar 29, 2024 •
edited
Loading

πŸ“Œ μž‘μ—… λ‚΄μš©

close #51

κ΅¬ν˜„ λ‚΄μš© 및 μž‘μ—… ν–ˆλ˜ λ‚΄μ—­

λ…Έμ…˜μ— μ˜¬λ €λ‘” ux κ°œμ„  λ‚΄μš©μ„ 기반으둜 기이수 κ³Όλͺ© μ‚­μ œ κΈ°λŠ₯을 λ¦¬νŒ©ν† λ§ν–ˆμŠ΅λ‹ˆλ‹€.
λͺ¨λ°”일 버전( μŠ€μ™€μ΄ν”„ ν•΄μ„œ μ‚­μ œ) κ³Ό pc 버전 ( hover ν–ˆμ„ λ•Œ μ‚­μ œ) λ₯Ό λ‘˜λ‹€ κ΅¬ν˜„ν–ˆκ³  mock을 λ§Œλ“€μ–΄ api 연결은 ν•˜μ§€ μ•Šμ•˜μŠ΅λ‹ˆλ‹€

(μ°Έκ³ )

  • button μ—μ„œ λ³€κ²½λœ 뢀뢄이 μžˆλŠ”λ° μ΄λŠ” μ–Έλ‹ˆ pr λ‚΄μš©μ„ κ°€μ Έμ˜¨κ±°λΌ λ¬΄μ‹œν•΄λ„ λ©λ‹ˆλ‹€
  • reactswipeable-list.css νŒŒμΌμ€ λΌμ΄λΈŒλŸ¬λ¦¬μ—μ„œ μ œκ³΅ν•΄μ£ΌλŠ” css이고 μ œκ°€ 쑰금 μ»€μŠ€ν…€ν•˜κΈ° μœ„ν•΄ distμ—μ„œ κ°€μ Έμ˜€λŠ” 게 μ•„λ‹Œ utils에 νŒŒμΌμ„ μƒμ„±ν–ˆμŠ΅λ‹ˆλ‹€.

πŸ€” κ³ λ―Ό ν–ˆλ˜ λΆ€λΆ„

μ–Έλ‹ˆκ°€ κ΅¬ν˜„ν•œ content container의 padding을 μ‚­μ œν–ˆμŠ΅λ‹ˆλ‹€

  • λͺ¨λ°”μΌμ—μ„œλŠ” padding 없이 taken lecture listλ₯Ό content에 꽉 μ±„μš°κ³  μ‹Άμ—ˆμŠ΅λ‹ˆλ‹€
  • μ–Έλ‹ˆκ°€ padding을 λ„£μ—ˆλ˜ μ˜λ„λŠ” λͺ¨λ“  contentμ—μ„œ padding이 ν•„μš”ν•  거라 μ˜ˆμƒλΌμ„œ 넣은 것 같은데, 그렇지 μ•Šμ€ κ²½μš°κ°€ μ‘΄μž¬ν•˜κ³  padding이 ν•„μš”ν•œ content의 경우 직접 padding 속성을 μΆ”κ°€ν•˜λ©΄ λ˜κΈ°μ— padding 속성 μ‚­μ œκ°€ 큰 λ¬Έμ œκ°€ μ•ˆλ  것 κ°™μŠ΅λ‹ˆλ‹€

Swipeable custom list λ₯Ό κ΅¬ν˜„ν–ˆμŠ΅λ‹ˆλ‹€

  • swipeable listκ°€ swipeable list item의 μƒμœ„μ— μžˆμ–΄μ•Ό 적용이 λ˜μ—ˆμŠ΅λ‹ˆλ‹€. List μ»΄ν¬λ„ŒνŠΈμ—μ„œ swipeableμ΄λΌλŠ” propsλ₯Ό λ°›μ•„μ„œ 쑰건뢀 λ Œλ”λ§μ„ ν•΄μ€„κΉŒλ„ κ³ λ―Όν–ˆμŠ΅λ‹ˆλ‹€
  • ν•˜μ§€λ§Œ List μ»΄ν¬λ„ŒνŠΈλŠ” λ‹¨μˆœνžˆ List ν˜•νƒœλ‘œ 정보λ₯Ό λ‚˜μ—΄ν•˜κΈ° μœ„ν•΄μ„œ λ§Œλ“€μ—ˆλŠ”λ°, List μ»΄ν¬λ„ŒνŠΈμ— swipe κΈ°λŠ₯도 μΆ”κ°€ν•˜λ©΄ μ—­ν• μ˜ λ²”μœ„κ°€ λ„˜μ–΄μ„°λ‹€κ³  μƒκ°ν•΄μ„œ λ”°λ‘œ κ΅¬ν˜„ν–ˆμŠ΅λ‹ˆλ‹€

gahyuun and others added 18 commits March 26, 2024 14:09
@gahyuun
refactor: add group style
09ef691
@gahyuun
chore: add hover style
49174d3
@seonghunYang
Merge pull request #50 from Myongji-Graduate/sign-up-form/#7
e44512e 
Sign up form/#7
@gahyuun
chore: merge commit
3927266
@gahyuun
feat: implement my page
af5ce65
@gahyuun
refactor: change button style
9c4bb78
@gahyuun
refactor: change table style
d7ef74c
@gahyuun
test: delete test code
1ab18ab
@gahyuun
refactor: delete custom logic
ba77e60
@gahyuun
refactor: change grid style
bf10b99
@gahyuun
style: implement table pc responsive design
4e41643
@gahyuun
chore: change custom grid style
8742ce5
@gahyuun
feat: implement table mobile responsive design
cb70e3d
@gahyuun
feat: add react-swipeable-list package
f4bc2e3
@gahyuun
feat: implement swipeable delete taken lecture button
9d6c6c8
@gahyuun
feat: implement swipeable list
3090005
@gahyuun
feat: add swipeable table to taken lecture list
9972291
@gahyuun
refactor: delete swipeable taken lecture button
7d5bb2c
@gahyuun gahyuun self-assigned this Mar 29, 2024
@github-actions GitHub Actions
Copy link

πŸš€storybook: https://65ccb85d5afe55a024495bc0-piugwkavwu.chromatic.com/

yougyung
yougyung reviewed Mar 29, 2024
View reviewed changes
Copy link
Member

@yougyung yougyung left a comment

Choose a reason for hiding this comment

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

reactswipeable-list.css νŒŒμΌμ€ λΌμ΄λΈŒλŸ¬λ¦¬μ—μ„œ μ œκ³΅ν•΄μ£ΌλŠ” css이고 μ œκ°€ 쑰금 μ»€μŠ€ν…€ν•˜κΈ° μœ„ν•΄ distμ—μ„œ κ°€μ Έμ˜€λŠ” 게 μ•„λ‹Œ utils에 νŒŒμΌμ„ μƒμ„±ν–ˆμŠ΅λ‹ˆλ‹€.

κ°€ν˜„μ΄κ°€ μ»€μŠ€ν…€μ„ μ§„ν–‰ν–ˆμ§€λ§Œ 순수 cssνŒŒμΌμ΄λΌλŠ” μ μ—μ„œ utils내뢀에 μœ„μΉ˜ν•˜λŠ” 것이 μ μ ˆν•œκ°€ 고민이 λ“­λ‹ˆλ‹€. app내에 style 폴더λ₯Ό μƒμ„±ν•΄μ„œ global.css와 ν•¨κ»˜ μœ„μΉ˜μ‹œν‚€λŠ” 것은 μ–΄λ–€κ°€μš”?

app/(sub-page)/my/page.tsx

export default function Page() {
return (
<ContentContainer className="flex">
Copy link
Member

Choose a reason for hiding this comment

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

lg:w-[npx]κ³Ό 같이 width값을 지정해쀀닀면, 방식이 λ°˜μ‘ν˜• κ΅¬ν˜„μ΄ 더 쉽고 UIμ μœΌλ‘œλ„ 보기 쒋을 것 κ°™μ•„μš”.

Copy link
Member Author

Choose a reason for hiding this comment

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

일단 page에 λŒ€ν•œ uiλ₯Ό μ§€κΈˆ 크게 μƒκ°ν•˜μ§„ μ•ŠκΈ΄ ν–ˆλŠ”λ°, ν˜Ήμ‹œ widthλ₯Ό 직접 μ§€μ •ν•΄μ£ΌλŠ” μ΄μœ μ— λŒ€ν•΄μ„œ 쑰금 더 μ„€λͺ…ν•΄μ£Όμ‹€ 수 μžˆμ„κΉŒμš”??
μ €λŠ” content containerμ—μ„œ width 70%둜 μ§€μ •ν•΄μ£ΌλŠ” 걸둜 μ•Œκ³  μžˆμ–΄μ„œμš”!

Copy link
Member

Choose a reason for hiding this comment

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

이 뢀뢄은 λŒ€λ©΄μœΌλ‘œ λ…Όμ˜ν•΄λ³΄λ©΄ 쒋을 것 κ°™μ•„μš”

app/(sub-page)/my/page.tsx Outdated
import ContentContainer from '@/app/ui/view/atom/content-container';
import React from 'react';

export default function Page() {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
export default function Page() {
export default function MyPage() {

Copy link
Member Author

Choose a reason for hiding this comment

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

μˆ˜μ •ν–ˆμŠ΅λ‹ˆλ‹€
16c72c4

app/ui/view/atom/label-container/label-container.tsx
@@ -8,7 +8,9 @@ interface LabelContainerProps {
export default function LabelContainer({ label, rightElement }: LabelContainerProps) {
return (
<div className="flex justify-between items-center w-full">
<div className="rounded-[100px] bg-light-blue-6 p-2.5 text-white text-lg font-bold">{label}</div>
<div className="rounded-[100px] bg-light-blue-6 p-2.5 text-white lg:text-sm xl:text-base 2xl:text-lg font-semibold">
Copy link
Member

Choose a reason for hiding this comment

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

lg:text-sm xl:text-base 2xl:text-lg λ°˜μ‘ν˜• μž‘μ—…ν•˜μ‹€λ•Œ, λͺ‡ pxκΉŒμ§€ κ³ λ €ν•˜λŠ” νŽΈμΈκ°€μš”?

Copy link
Member Author

Choose a reason for hiding this comment

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

breakpoint에 λŒ€ν•œ 말씀 λ§žμœΌμ‹€κΉŒμš”?
λ””μžμ΄λ„ˆκ°€ μ—†λ‹€λŠ” κ°€μ •ν•˜μ— μ»΄ν¬λ„ŒνŠΈ κ΅¬ν˜„ν•΄λ³΄λ©΄μ„œ μ»΄ν¬λ„ŒνŠΈλ§ˆλ‹€ λ‹€λ₯΄κ²Œ μ μš©ν•˜λŠ” 것 κ°™μŠ΅λ‹ˆλ‹€.
예λ₯Ό λ“€μ–΄ μ–΄λ– ν•œ μ»΄ν¬λ„ŒνŠΈλŠ” 768px μ΄ν•˜μΌλ•ŒλŠ” 같은 uiλ₯Ό κ°€μ Έκ°€λŠ” 반면, μ–΄λ– ν•œ μ»΄ν¬λ„ŒνŠΈλŠ” 640px μ΄ν•˜μΌλ•Œ, 640~768px일 λ•Œλ₯Ό λ‚˜λˆ„λŠ” 것 κ°™μŠ΅λ‹ˆλ‹€

app/ui/view/molecule/list/swipeable-custom-list.tsx
export default function SwipeableCustomList<T extends ListRow>({ data, render }: SwipeableListProps<T>) {
return (
<div className="rounded-xl border-[1px] border-gray-300 w-full ">
<SwipeableList type={ListType.IOS}>{data.map((item, index) => render(item, index))}</SwipeableList>
Copy link
Member

Choose a reason for hiding this comment

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

key값이 λΆ€μ—¬λ˜μ§€μ•Šμ•˜λ‹€λŠ” warning이 λœ¨κ³ μžˆλ„€μš”

Copy link
Member Author

Choose a reason for hiding this comment

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

μ•Œλ €μ£Όμ…”μ„œ κ°μ‚¬ν•©λ‹ˆλ‹€!
7990bcf

seonghunYang
seonghunYang reviewed Mar 29, 2024
View reviewed changes
app/ui/lecture/taken-lecture/taken-lecture-list.tsx
Comment on lines +15 to +31
{/* pc */}
<div className="hidden lg:block">
<Table
headerInfo={headerInfo}
data={customLecture}
data={data}
renderActionButton={(id: number) => <DeleteTakenLectureButton lectureId={id} />}
/>
) : (
<Table headerInfo={headerInfo} data={data} />
)}
</div>
{/* mobile */}
<div className="block lg:hidden">
<Table
headerInfo={headerInfo}
data={data}
swipeable={true}
renderActionButton={(id: number) => <DeleteTakenLectureButton lectureId={id} swipeable={true} />}
/>
</div>
Copy link
Collaborator

Choose a reason for hiding this comment

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

  • CSS의 display:none 속성을 μ΄μš©ν•΄ λ°˜μ‘ν˜•μ„ κ΅¬ν˜„ν•œ νŠΉλ³„ν•œ μ΄μœ κ°€ μžˆλ‚˜μš”?
  • 이 방식을 μ‚¬μš©ν•˜λ©΄ λͺ¨λ°”μΌμ—μ„œλŠ” 보이지 μ•Šμ§€λ§Œ PC용 DOM을 계속 가지고 μžˆμ–΄μ•Ό ν•˜λŠ” 단점이 μžˆμŠ΅λ‹ˆλ‹€.
  • ν•΄λ‹Ή μ»΄ν¬λ„ŒνŠΈκ°€ 이미 ν΄λΌμ΄μ–ΈνŠΈ μ»΄ν¬λ„ŒνŠΈμ΄λ―€λ‘œ, λ°˜μ‘ν˜• κ΄€λ ¨ 훅을 λ§Œλ“€μ–΄ JS둜 λ°˜μ‘ν˜•μ„ κ΄€λ¦¬ν•˜λŠ” 것은 μ–΄λ–¨κΉŒμš”?

Copy link
Member Author

Choose a reason for hiding this comment

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

useMediaQuery 같은 κ±Έ λ§Œλ“€μ–΄μ„œ λ°˜μ‘ν˜•μœΌλ‘œ 관리할 μˆ˜λ„ μžˆμ„ 것 κ°™λ„€μš”!

그러면 λ°˜μ‘ν˜• ν›…μœΌλ‘œ 인해 μ„œλ²„ μ»΄ν¬λ„ŒνŠΈκ°€ ν΄λΌμ΄μ–ΈνŠΈ μ»΄ν¬λ„ŒνŠΈλ‘œ λ³€κ²½λ˜λŠ” κ²½μš°λ„ μžˆμ„ 것 같은데, 이거에 λŒ€ν•œ λΉ„μš©λ³΄λ‹€ pc용 dom을 가지고 μžˆλŠ” 것이 더 큰 λΉ„μš©μΈμ§€ κΆκΈˆν•©λ‹ˆλ‹€! ( ex λ°˜μ‘ν˜• ν›… λ‚΄λΆ€μ—μ„œ useState μ‚¬μš© )

Copy link
Collaborator

Choose a reason for hiding this comment

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

  • λŒ€λΆ€λΆ„μ˜ λ°˜μ‘ν˜• μœ μŠ€μΌ€μ΄μŠ€λŠ” tailwind의 Breakpoint prefixλ₯Ό μ‚¬μš©ν•˜μ—¬ 관리할 수 μžˆμ„ 것 κ°™μ•„μš”
  • κ·ΈλŸ¬λ‚˜, ν˜„μž¬μ™€ 같이 λ°˜μ‘ν˜•μœΌλ‘œ 인해 μ»΄ν¬λ„ŒνŠΈλ‚˜ λ ˆμ΄μ•„μ›ƒμ΄ λ³€ν•˜λŠ” κ²½μš°μ—λŠ” λΆˆν•„μš”ν•œ DOM νŒŒμ‹±μ„ ν”Όν•˜κΈ° μœ„ν•΄ 더 μš°μ•„ν•œ 방법이 ν•„μš”ν•˜μ§€ μ•Šμ„κΉŒ ν•˜λŠ”λ° 제 μƒκ°μž…λ‹ˆλ‹€.
  • λ§μ”€ν•˜μ‹ λŒ€λ‘œ hook을 μ‚¬μš©ν•˜μ—¬ κ΄€λ¦¬ν•˜λ©΄, ν•΄λ‹Ή μ»΄ν¬λ„ŒνŠΈκ°€ λ³€ν•΄μ•Ό ν•˜λŠ” ν΄λΌμ΄μ–ΈνŠΈ μ»΄ν¬λ„ŒνŠΈ λ¬Έμ œκ°€ λ°œμƒν•  수 μžˆκ² λ„€μš”
  • 이에 λŒ€ν•΄ κ³ λ―Όν•΄ λ³΄μ•˜λŠ”λ°, 이전에 μ–ΈκΈ‰ν–ˆλ˜ ꡬ성 νŒ¨ν„΄μ„ μ΄μš©ν•˜μ—¬ λ°˜μ‘ν˜•μ„ λ‹΄λ‹Ήν•˜λŠ” ν΄λΌμ΄μ–ΈνŠΈ μ»΄ν¬λ„ŒνŠΈλ₯Ό λ§Œλ“€μ–΄ μ„ μ–Έμ μœΌλ‘œ μ²˜λ¦¬ν•  수 μžˆμ„ 것 κ°™μŠ΅λ‹ˆλ‹€.
  • 이 λ¬Έμ œμ— λŒ€ν•΄ 저도 더 κ³ λ―Όν•΄ λ³΄κ² μŠ΅λ‹ˆλ‹€. 일단 이 PR은 λ³‘ν•©ν•˜κ³ , 회고 μ‹œκ°„μ— 더 μžμ„Ένžˆ μ΄μ•ΌκΈ°ν•΄λ΄…μ‹œλ‹€

ꡬ성 νŒ¨ν„΄: https://nextjs.org/docs/app/building-your-application/rendering/composition-patterns#supported-pattern-passing-server-components-to-client-components-as-props

Copy link
Member Author

@gahyuun gahyuun Apr 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

λ„΅ 저도 κ³ λ―Ό ν•΄λ³΄κ² μŠ΅λ‹ˆλ‹€!

app/utils/style/reactswipeable-list.css Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

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

[comment]
제 μƒκ°μ—λŠ” CSS νŒŒμΌμ€ μœ ν‹Έλ³΄λ‹€λŠ” 정적 νŒŒμΌμ΄λ―€λ‘œ public에 μœ„μΉ˜ν•˜λŠ” 것이 μ’‹μ•„λ³΄μ΄λ„€μš”

app/ui/view/molecule/list/swipeable-custom-list.tsx
Copy link
Collaborator

Choose a reason for hiding this comment

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

[comment]

  • 저도 list의 속성을 μΆ”κ°€ν•˜λŠ” 것보닀 SwipableCustomListλ₯Ό κ΅¬ν˜„ν•˜λŠ”κ²Œ 더 μ’‹μ•„ λ³΄μ΄λ„€μš”
  • μ΄μœ λŠ” κΈ°μ‘΄ μ½”λ“œλ₯Ό μˆ˜μ •ν•˜μ§€ μ•Šκ³  κΈ°λŠ₯을 ν™•μž₯ν–ˆκΈ° λ•Œλ¬Έμ— OCPλ₯Ό μΆ©μ‘±μ‹œν‚€κΈ° λ•Œλ¬Έμž…λ‹ˆλ‹€

@github-actions GitHub Actions
Copy link

πŸš€storybook: https://65ccb85d5afe55a024495bc0-afjyopupij.chromatic.com/

yougyung
yougyung previously approved these changes Mar 31, 2024
View reviewed changes
@github-actions GitHub Actions
Copy link

github-actions bot commented Apr 1, 2024

πŸš€storybook: https://65ccb85d5afe55a024495bc0-kaaicudact.chromatic.com/

@gahyuun
Copy link
Member Author

gahyuun commented Apr 1, 2024

@seonghunYang @yougyung
css νŒŒμΌμ€ global.css λž‘ λ™μΌν•œ μœ„μΉ˜, app 폴더 μ΅œμƒμœ„μ— μœ„μΉ˜ν•˜λ„λ‘ μˆ˜μ •ν–ˆμŠ΅λ‹ˆλ‹€. μ΄μœ λŠ” λ‹€μŒκ³Ό κ°™μŠ΅λ‹ˆλ‹€!

  1. μ™ΈλΆ€ νŒ¨ν‚€μ§€ public 폴더에 μœ„μΉ˜μ‹œν‚€λ €ν–ˆμœΌλ‚˜, μΆ”ν›„ 또 μ»€μŠ€ν…€μ„ μ§„ν–‰ν•˜κ³  μ‹Άμ–΄ css 파일 λ‚΄ μ½”λ“œλ₯Ό λ³€κ²½μ‹œν‚¬ κ°€λŠ₯성이 μžˆλ‹€κ³  νŒλ‹¨ν–ˆκ³  κ·Έλ ‡λ‹€λ©΄ assetμ΄λ‚˜ λ³€κ²½λ˜μ§€ μ•Šμ„ 파일인 mockserviceworker.jsκ°€ μ‘΄μž¬ν•˜λŠ” Public 폴더가 μ—„μ²­ 적합해보이진 μ•Šμ•˜μŠ΅λ‹ˆλ‹€!
  2. app 폴더 λ‚΄μ˜ style 폴더λ₯Ό λ§Œλ“€μ–΄μ„œ global.css와 ν•¨κ»˜ μœ„μΉ˜μ‹œν‚¬κΉŒλ„ κ³ λ―Όν–ˆμ§€λ§Œ, 두 파일 밖에 μ‘΄μž¬ν•˜μ§€ μ•ŠκΈ°μ— ꡳ이 뎁슀λ₯Ό λ§Œλ“€μ§„ μ•Šμ•˜μŠ΅λ‹ˆλ‹€!

seonghunYang
seonghunYang approved these changes Apr 1, 2024
View reviewed changes
Copy link
Collaborator

@seonghunYang seonghunYang left a comment

Choose a reason for hiding this comment

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

  • ν™•μΈν–ˆμ–΄μš”. μˆ˜κ³ ν•˜μ…¨μŠ΅λ‹ˆλ‹€

@gahyuun gahyuun merged commit dd277b4 into main Apr 1, 2024
3 checks passed
@gahyuun gahyuun deleted the refactor-delete/#55 branch April 1, 2024 11:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@seonghunYang seonghunYang seonghunYang approved these changes

@yougyung yougyung yougyung approved these changes

Assignees

@gahyuun gahyuun

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

κ³Όλͺ© μ‚­μ œ λ¦¬νŒ©ν† λ§
3 participants
@gahyuun @seonghunYang @yougyung