Feature/user type #13
Conversation
학생 전형 페이지의 각 줄들입니다.
학생 전형 페이지 스타일
까먹고 안올렸습니다
| import { TypeElement, TypeElementName } from '../../../styles/ChoiceType'; | ||
|
|
||
| interface Props { | ||
| children: JSX.Element, |
There was a problem hiding this comment.
| children: JSX.Element, | |
| children: React.ReactNode, |
아래처럼 쓰면 에러남 ㅇㅇ
<DefaultRow>
<Component />
<Component />
</DefaultRow>
There was a problem hiding this comment.
아앗..몰랐습니다 React.ReactNode라는 속성을 못찾았었습니다,
There was a problem hiding this comment.
참고로 저거 props에 추가 안해줘도 FC에서 기본적으로 props에 담고있음
There was a problem hiding this comment.
아 children 말하시는거죠?
확실하게 React.ReactNode만 오게 하려고 일부로 하긴 했습니다만..
어떻게 하는게 좋을까요?
| import { DefaultRow } from '..'; | ||
|
|
||
| interface Props { | ||
| describe?: string, |
There was a problem hiding this comment.
description이 맞을듯? 뭐 네이밍은 하기나름인데 기왕이면 명확하게 명사로
There was a problem hiding this comment.
아 저거 description 치다가 중간에 뭔 일 있어서 describe 치고 한거라 ㅋㅋ
수정하겠습니다.
| <Radio | ||
| radioName="district" | ||
| value="대전" | ||
| valueChangeHandler={valueChangeHandler} | ||
| >대전</Radio> | ||
| <Radio | ||
| radioName="district" | ||
| value="전국" | ||
| valueChangeHandler={valueChangeHandler} | ||
| >전국</Radio> |
There was a problem hiding this comment.
radio 인터페이스가 좀 이상한데?
일단 checked를 받아야하고.. 네이밍도 valueChange가 아니라 그냥 onChange라고 해야할듯
왜냐면 radio인터페이스에서 의미하는 value는 checked랑 다르니까.. 근데 사실이건 html스펙이 문제인듯 ㅋㅋ 인터페이스 개구림
<Radio
name="district"
label="대전"
value="대전"
checked={value === "대전"}
onChange={e => valueChangeHandler("대전")}
/>
<Radio
name="district"
label="전국"
value="전국"
checked={value === "전국"}
onChange={e => valueChangeHandler("전국")}
/>
이정도면 충분한 인터페이스일것 같은데, 나같으면
<RadioGroup value={value} onChange={valueChangeHandler}>
<RadioItem
value="대전"
label="대전"
/>
<RadioItem
value="대전"
label="대전"
/>
</RadioGroup>
context 사용해서 위 인터페이스처럼 구현하겠음 value와 label로 이루어진 radio item들이 있고, 이 엘리먼트들에 바운더리를 두어서 context가 자식 radio들을 array처럼 가지는거임 그럼 얘들끼리 묶이니까 name 필드도 주입해줄 필요없고..
There was a problem hiding this comment.
생각해보니, label 을 따로 두는게 맞는 코드인 것 같네요
그리고 저렇게 Group으로 묶으니 서로 묶여있는 element라는 느낌이 강해 보기 편하네요
감사합니다!
| <DropdownRadio | ||
| valueChangeHandler={valueChangeHandler} | ||
| radioName="type" | ||
| value={getCheckedMenu(otherTypeList).VALUE} |
There was a problem hiding this comment.
?? 이렇게 할필요없이
[
"사회통합전형",
"기초생활수급자",
"한부모가족",
"차상위계층",
"소년소녀가장",
"북한이탈주민",
"다문화가정",
]
이걸 dropdown에 넘기고, onChange이벤트로 "사회통합전형"같은 value를 넘겨주면 되죠??
그리고 아마 서버에서 "사회통합전형" 이렇게 korean literal로 안받을거같긴한데..?
There was a problem hiding this comment.
아 이건 서버 나오기 전에 잠깐 놔둔거긴 합니다만, 저렇게 하는건 생각 못했네요 수정하겠습니다!
There was a problem hiding this comment.
다른 defaultElement들도 다 수정해야겠네요..
감사합니다!
There was a problem hiding this comment.
아 그리고 value와 label(보여줄 글자)를 다르게 해서 만드는 것이 좋겠..죠?
There was a problem hiding this comment.
ㅇㅇㅇㅇ 아마 서버에서 데이터 만든거 참고하면될듯? ㅋㅋ 19년도에 차상위계층 CHA_UPPER 이딴거했던거같은데 ㅋㅋㅋㅋㅋㅋ
There was a problem hiding this comment.
ㅋㅋ cha upper chacha upper from north 이런거?
There was a problem hiding this comment.
ㅋㅋㅋㅋㅋㅋㅋ 지금 보니 개웃기네 from north
나름 최선이었음 ㅋㅋㅋ
| <Dropdown | ||
| valueChangeHandler={valueChangeHandler} | ||
| setList={setYearList} | ||
| menuList={graduationYearList} | ||
| /> |
| <TypeElementContent> | ||
| <div> | ||
| <Checkbox | ||
| valueChangeHandler={nationalMeritChangeHandler} |
There was a problem hiding this comment.
checkbox 인터페이스도 수정해야할듯.. state를 부모에서 관리해야함. 자식에서 그러니까 checked를 받아야함
https://reactjs.org/docs/forms.html#handling-multiple-inputs
There was a problem hiding this comment.
그럼 checked를 넣는 이유는 checkbox가 체크 됬나 안됬나를 부모 component에서 가지고 있기 위해서인가요? 가지고 있어야 하는 이유를 잘 모르겠습니다.
There was a problem hiding this comment.
개발하다가 다시 생각해보니, 나중에 적용될 저장된 것 로드해서 올 때, checked를 바꿔야 하기 때문에 필요한 것이군요. 충고 감사합니다!!
| GraduationYear, | ||
| Specialty, | ||
| } from './RowType'; | ||
|
|
There was a problem hiding this comment.
그냥 의견이긴 한데
// import & export
export { DefaultRow } from './RowType/defaultRow';이런식으로 한 줄로 적는거 가능해요.
드롭다운 interface 수정으로 인한 변화 및 dropdown list를 constance file 로 따로 빼냄
checkbox를 자동저장된 정보를 다시 로드할 수 있도록 check를 부모에서 조정할 수 있도록 interface 수정
input에 자동저장된 로그를 불러올 시 적용될 수 있게 value를 부모에서 수정 가능하게 바꿈
| const RadioGroupProvider: FC<Props> = ({ | ||
| children, | ||
| onChange, | ||
| name, | ||
| }) => { |
There was a problem hiding this comment.
전에 말했던 controlled component문제. value를 받아야함
그리고 name을.. 직접받아도 상관은없는데 사실 저거 각 radio group마다 unique id가 필요한거잖음 결국
facebook/react#17322
useOpaqueIdentifier 써보진않았는데 대충 이런거로 해결하면 굳이 name받을 필요없을듯?
| interface ContextValue { | ||
| onChange: Function, | ||
| name: string, | ||
| } | ||
|
|
||
| interface Props { | ||
| onChange: Function, | ||
| children: React.ReactNode, | ||
| name: string, | ||
| } |
There was a problem hiding this comment.
사실 엔트리의 경우에는 항상 value: string이겠지만 이런 일반적인 컴포넌트를 만드는경우 재상용성을 높이려면 value를 generic으로 받아야함. 그래서 사용할때
<Radio value={1}>label</Radio>이렇게도 사용할 수 있도록.
그리고 onChange에서 타입이 누락되었던데 만약 제네릭으로 한다면
onChange?: (value: T) => void이런식으로 해야할거고 (optional인 이유도 마찬가지. 조금 더 general한 인터페이스), 하지만 지금같은경우는 string으로 고정해도 문제될게없어보임
onChange?: (value: string) => voidThere was a problem hiding this comment.
엔트리 같은 경우에 값이 나올 수 있는 경우가 string 밖에 없다고 생각해서
제네릭으로 만들 필요가 없다고 판단을 하긴 했는데..이건 제네릭으로 처리하는게 좋을 것 같습니다. 감사합니다 수정하도록 하겠습니다!!
There was a problem hiding this comment.
ㄴㄴㄴ 일반적인 컴포넌트를 만드려면 그렇게 해야한다고 알려준거였는데 그렇게 이유가있던거면 꼭 수정할필요는 없어보임
There was a problem hiding this comment.
사실 엔트리의 경우에는 항상 value: string이겠지만 이런 일반적인 컴포넌트를 만드는경우 재상용성을 높이려면 value를 generic으로 받아야함. 그래서 사용할때
<Radio value={1}>label</Radio>이렇게도 사용할 수 있도록.
그리고 onChange에서 타입이 누락되었던데 만약 제네릭으로 한다면onChange?: (value: T) => void이런식으로 해야할거고 (optional인 이유도 마찬가지. 조금 더 general한 인터페이스), 하지만 지금같은경우는 string으로 고정해도 문제될게없어보임
onChange?: (value: string) => void
질문이 있습니다.
만약에 이렇게 제네릭으로 처리할 경우에, 값들을 redux에 넘겨줄 때 타입을 제네릭 변수로만 알 수 있게 되니 redux의 reducer에서 설정해준 state들을 모두 제네릭으로 바꾸는 게 맞을까요?
| const contextValue: ContextValue = { | ||
| onChange: ()=>{}, | ||
| name: "", | ||
| } | ||
|
|
||
| const RadioGroupContext = createContext(contextValue); |
There was a problem hiding this comment.
이것도 방법이있는데 지금 강제하고자하는 인터페이스가
<Radio />는 항상 <RadioContextProvider />의 자손에서 사용된다는거잖음??
그럼 context default value로 null을 넣어놓고 Radio component에서 useContext를 할때 에러를 내버리는거지
const context = useContext(RadioGroupContext);
if (!context) {
throw Error("radio group context must be provieded");
}공용컴포넌트를 만들때는 이런식으로 인터페이스사용을 유도할 수 있음
| interface menuList { | ||
| VALUE: string, | ||
| isChecked: boolean, | ||
| LABEL: string, | ||
| } |
There was a problem hiding this comment.
drop down value도 마찬가지. 이경우 제네릭으로 만들어두면 더 일반적으로 사용가능
|
|
||
| interface menuList { | ||
| VALUE: string, | ||
| isChecked: boolean, | ||
| LABEL: string, | ||
| } | ||
|
|
||
| interface Props { | ||
| width?: string, | ||
| menuList: menuList[], | ||
| isAble?: boolean, |
There was a problem hiding this comment.
아 이게..dropdown이 동작이 되어야 할때와 안되야 할때가 있습니다.
예를 들면, radio가 checked여야지만 펼쳐져야 하는 dropdown이 있어야 해서 만든겁니다.
다른 방법이 딱히 떠오르지 않아서 props를 하나 더 추가했습니다.
| setList: (list: menuList[]) => void, | ||
| valueChangeHandler: (value: string) => void, | ||
| onChange: (value: string) => void, | ||
| savedValue?: string, |
There was a problem hiding this comment.
saved value? 무슨뜻이지?? 그냥 value로 써도되는거아닌가
There was a problem hiding this comment.
그..자동저장 후에 불러오는 data임을 강조하기 위해서 savedValue라고 했습니다.
value로 해도 될 것 같아요. 수정하겠습니다.
| interface Props { | ||
| width?: string, | ||
| menuList: menuList[], | ||
| isAble?: boolean, | ||
| setList: (list: menuList[]) => void, | ||
| valueChangeHandler: (value: string) => void, | ||
| onChange: (value: string) => void, | ||
| savedValue?: string, |
There was a problem hiding this comment.
dropdown은
onChange?: (value: string) => void;
value?: string;
options: {label: string; value: string}[];요정도가 적절할것같은데.. 그리고 기타 인터페이스(width나 placeholder.. disabled 이런거)는 필요하면 열어두고
There was a problem hiding this comment.
네이밍은 여전히..직관적인 이름 짓기가 좀 애매하네요
이름을 수정하도록 하겠습니다!!
그리고 component를 다르게 사용하면 에러 메세지 발생시킴 ,그리고 제네릭으로 변경
| children, | ||
| onChange, | ||
| }: Props<T>){ | ||
| const hashedName = getHashName(getRandomString()); |
There was a problem hiding this comment.
random string이 겹치는게 문제라면 그냥 auto increment훅을 하나 만들어서 해결하면 될듯
그리고 memoize필요할듯
const name = useMemo(() => getName(), [])
혹은 이렇게 이걸 아에 useUniqueId라고 커스텀훅으로 만들어도될듯. 근데 다른곳에서 쓰일 여지는 없어보여서 패스..
There was a problem hiding this comment.
memo 해야되네요.. 시정하겠습니다.
auto increment훅이 뭔지 이해가 잘 가질 않습니다.
자동으로 늘어나는 훅...?이 맞나요?
There was a problem hiding this comment.
? 제정신으로 쓴게아니구만 ㅋㅋ 설명을 왜저렇게했는지 모르겠지만 그냥 getName 할때마다 0, 1, 2.. 리턴되도록
There was a problem hiding this comment.
요번에 commit한 increment함수를 구현하긴 했는데, 이상하다고 생각합니다..
더 나은 방법을 제시해주실 수 있을까요?
| <RadioGroupProvider | ||
| onChange={valueChangeHandler} | ||
| > |
There was a problem hiding this comment.
radio에서 여전히 value를 안받는거같은데?? (서버에서 불러온 데이터 렌더링할때 생각하셈)
There was a problem hiding this comment.
아 group 적용 전에 value를 받게 해놔서 그랬던 거 같네요 시정하겠습니당..
There was a problem hiding this comment.
요번에 radioDropdown을 위해서 추가된 props가 2개가 있는데,
nowDropdownValue와 options가 추가 되었어요.
radioDropdown을 클릭해도 dropdown의 값을 끌어와서 state를 변경하기 위해서와
checked를 구현하기 위해서 props를 추가하였으니 참고해주시길 바랍니다~
| if (!onChange) { | ||
| throw Error("radio must grouped by radio group"); | ||
| } |
There was a problem hiding this comment.
onChange에 대한 검사가 왜필요하지?? 그리고 검사내용이랑 에러내용이랑 안맞는듯
There was a problem hiding this comment.
어..제가 왜저랬는지는 잘 모르겠습니다.
검사 제대로 하게 수정하겠습니다.
There was a problem hiding this comment.
생각해보니 useRadioGroupContext()// context return 하는 녀석에서 error check를 해주니
똑같은 check를 여기서 해줄 필요가 없다는걸 깨닫고 if문을 없앴습니다!
| function useRadioGroupContext<T>() { | ||
| const context = useContext(RadioGroupContext); | ||
| if (!context) { | ||
| throw Error |
There was a problem hiding this comment.
이거..는 내가 설명할때 간략하게 표현한거고
throw new Error("radio group context must be provided")
라고 해줘야겠죠
There was a problem hiding this comment.
제가 무슨 정신으로 저걸 저렇게 나뒀는지 모르겠습니다.
시정하겠습니다
| /> | ||
| <div></div> | ||
| <div/> |
There was a problem hiding this comment.
음..css적인 요소때문에 넣어놓은 녀석이였습니다.
css를 수정하도록 하겠습니다
There was a problem hiding this comment.
저 div가 web 상에서 실제로 표현되는 radio 버튼인데, 명시를 안해놔서 헷갈릴 수 있었다고 생각합니다. 문제점 파악 후, className 추가하여 역할 명시 하도록 하겠습니다.
There was a problem hiding this comment.
따로 설명해뒀지만, 여기에도 달자면 이렇게 logic과 분리돼서 view에만 영향을 주는 요소라면 ::after같은 가상선택자로해결가능
There was a problem hiding this comment.
생각해보니 div로 after를 사용하고 있었는데 그냥 input에다가 ::before ::after로 하면 됬던것 같네요. 시정하도록 하겠습니다. 감사합니다
| const isChecked = (valueList:{ LABEL: string, VALUE: string }[], savedValue:T) => { | ||
| let flag = false; | ||
| valueList.map((value)=> { | ||
| const convertedValue = savedValue as unknown as string; |
There was a problem hiding this comment.
어차피 이렇게 되는상황인거면 제네릭으로 하면 안되지.. 걍 string으로 고정하셈 dropdown radio만
| let flag = false; | ||
| valueList.map((value)=> { | ||
| const convertedValue = savedValue as unknown as string; | ||
| if(value.VALUE === convertedValue){ | ||
| flag = true; | ||
| } | ||
| }) | ||
| return flag; |
There was a problem hiding this comment.
| let flag = false; | |
| valueList.map((value)=> { | |
| const convertedValue = savedValue as unknown as string; | |
| if(value.VALUE === convertedValue){ | |
| flag = true; | |
| } | |
| }) | |
| return flag; | |
| const convertedValue = savedValue as unknown as string; | |
| return valueList.some((value)=> value.VALUE === convertedValue) |
There was a problem hiding this comment.
some이란 함수를 처음보는데, 유용한 함수 인 것 같네요.
알려주셔서 감사해요!
| name, | ||
| savedValue, | ||
| }:ContextValue<T> = context; | ||
| const isChecked = (valueList:{ LABEL: string, VALUE: string }[], savedValue:T) => { |
There was a problem hiding this comment.
그리고 애초에 이 함수는 rendering blcok에 있을필요도없음. 바깥으로 뺍시다
| valueChangeHandler: Function, | ||
| value: string, | ||
| interface Props<T> { | ||
| value: T, | ||
| ableChange: Function, |
There was a problem hiding this comment.
아직 네이밍에 익숙하지 못한 것 같습니다.
알맞은 이름 고민 후 시정하겠습니다.
유저 전형 선택 페이지
목적
유저들이 전형을 선택 할 수 있게 하기 위함이다.
요약
유저들 전형 페이지의 각 줄들을 마크업 및 기능을 구현
상세
유저들 전형 페이지의 각 줄을 마크업 하고
그에 따른 기능들을 추가하습니다.
영향을 미치는 부분
없습니다.
참조(선택)
없습니다.