Conversation
HakSooDev
left a comment
HakSooDev
left a comment
There was a problem hiding this comment.
커밋할때 자동 linting 이 돌아갈텐데 안돌아가있네요. 한번 확인해주시겠어요?
고생하셨습니다.
| return ( | ||
| <SidebarProvider> | ||
| <div className="flex h-screen bg-background text-foreground"> | ||
| <div className="flex h-screen w-full bg-background text-foreground"> |
There was a problem hiding this comment.
flex size-full bg-background text-foreground
이면 충분할것 같네요
frontend/src/app/page.tsx
Outdated
| <div> | ||
| <h1>Light Switch</h1> | ||
| <div className="flex w-full flex-col gap-5 p-3"> | ||
| <header className="flex w-[270px] flex-row items-center justify-between"> |
There was a problem hiding this comment.
특별한 경우가 아니라면 px 지정은 안해주는게 좋을것 같네요.
px 을 적어놓으면 나중에 layout 바뀌면 바로 깨져요.
flex-row 도 default 값이여서 생략 가능합니다.
className="flex items-center gap-3" 이면 충분할것 같습니다.
frontend/src/app/page.tsx
Outdated
| <div className="flex w-full flex-col gap-5 p-3"> | ||
| <header className="flex w-[270px] flex-row items-center justify-between"> | ||
| <FlagIcon size={20} /> | ||
| <span className="text-[1.2rem] font-semibold"> |
There was a problem hiding this comment.
text-[1.2rem] => text-xl
frontend/src/app/page.tsx
Outdated
| </span> | ||
| </header> | ||
|
|
||
| <DataTable columns={columns} data={ExampleData as TableDataType[]} /> |
There was a problem hiding this comment.
as 는 되도록이면 안쓰는게 좋습니다.
as TableDataType[] 지우셔도 될것 같습니다.
frontend/src/types/home.ts
Outdated
| @@ -0,0 +1,8 @@ | |||
| export type TableDataType = { | |||
There was a problem hiding this comment.
타입 fields 들은 다 camelCase 여야 합니다.
| variant="ghost" | ||
| onClick={() => column.toggleSorting(column.getIsSorted() === 'asc')} | ||
| > | ||
| Name |
There was a problem hiding this comment.
Name 이 아니고 Key 로 해야할것 같습니다. 관련해서 모두 Key 로 바꿔주세요.
| header: 'By', | ||
| }, | ||
| { | ||
| accessorKey: 'Status', |
There was a problem hiding this comment.
Status 는 ToggleButton 으로 하는게 좋을것 같습니다.
| header: 'Status', | ||
| }, | ||
| { | ||
| accessorKey: 'Default Value', |
There was a problem hiding this comment.
table 에서 default value 를 보여줄필요는 없을것 같습니다.
| @@ -0,0 +1,78 @@ | |||
| 'use client'; | |||
There was a problem hiding this comment.
이친구는 component가 아니라서 일단은 components/featureFlagColumns.tsx 로 하는게 좋을것 같습니다..
There was a problem hiding this comment.
헤더에 React 요소를 반환중이라 use client가 필요했는데 sorting 기능 사용하지 않을 예정이니 모두 삭제하고 string만 반환하게 바꾸겠습니다.
frontend/src/types/home.ts
Outdated
| @@ -0,0 +1,8 @@ | |||
| export type TableDataType = { | |||
There was a problem hiding this comment.
type 들은 data 에따라서 type 이 나뉘어야지 페이지에따라서 type 을 지정하면 안될것 같습니다.
일단 types.ts 만들어서 FeatureFlagTableType 으로 넣어놓는게 좋을것 같습니다.
|
피드백 관련해서 모두 수정해서 올려놨습니다 |
| @@ -1,116 +1,102 @@ | |||
| import { TableDataType } from '@/types/home'; | |||
| import { featureFlagTableType } from '@/types/types'; | |||
frontend/src/types/types.ts
Outdated
| @@ -0,0 +1,7 @@ | |||
| export type featureFlagTableType = { | |||
There was a problem hiding this comment.
보통 Type 이름은 pascal case 고 fields 들은 camel case 입니다.
frontend/src/types/types.ts
Outdated
| Type: string; | ||
| Created: string; | ||
| By: string; | ||
| ToggleButton: boolean; |
There was a problem hiding this comment.
ToggleButton 아니고 active 나 이런거 일것 같은데 백엔드 타입 한번 확인해보시겠어요?
| @@ -0,0 +1,28 @@ | |||
| 'use client'; | |||
There was a problem hiding this comment.
components/ui 에는 shared UI component 가 들어가야합니다.
이친구는 Home 에서만 쓰이면 거기 안에만 들어가있어도 충분하지 않을까요?
| @@ -1,17 +1,14 @@ | |||
| 'use client'; | |||
There was a problem hiding this comment.
이친구는 components/ 에 들어가는게 더 맞을것 같습니다.
| } | ||
|
|
||
| export function DataTable<TData extends TableDataType, TValue>({ | ||
| export function DataTable<TData extends featureFlagTableType, TValue>({ |
| variant="outline" | ||
| size="sm" | ||
| onClick={handlePreviousPage} | ||
| onClick={() => table.previousPage()} |
There was a problem hiding this comment.
() => table.previousPage() => table.previousPage
3 participants
Feedback from another team member
Accordingly, the decision has been made to restructure the table using mockData and submit it again."