Fix disabled button UI #394
Conversation
✅ Deploy Preview for biseo-preview ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
prettier 포맷팅이 잘 안 된것 같아요! |
|
#393 PR과 같은 브랜치에서 작업 중이신것 같은데, 새로운 작업 하실 때에는 새로운 브랜치에서 작업해 주셔야 해요! PR 하나당 브랜치 하나가 1대1 대응된다고 생각하시면 될 것 같아요 |
| </Text> | ||
| { | ||
| validated ? ( | ||
| <Text variant="boldtitle3" color="blue600"> |
There was a problem hiding this comment.
| <Text variant="boldtitle3" color="blue600"> | |
| <Text variant="boldtitle3" color={validated ? "blue600" : "gray어쩌고"}> |
바뀌는 게 컬러 뿐인 것 같아서 요렇게 하면 코드 중복도 줄고 더 깔끔할 것 같아요!!!
|
근데 제가 건든 파일이 createAgendaModal.tsx랑 button.tsx밖에 없는데 prettier는 왜 안 될까요 ㅜ.. |
이 파일 문제라는데, 혹시 vscode에 prettier 작동하게 설정되어 있나요? vscode settings 열어서 "formatter" 검색해서 prettier 선택되어 있는지, format on save 선택되어 있는지 확인해주세요! |
| <Button w={270} h={38} onClick={onSubmit} disabled={!validated}> | ||
| <Text variant="boldtitle3" color="blue600"> | ||
| <Text variant="boldtitle3" color={validated? "blue600" : "gray300"}> | ||
| 투표 생성하기 | ||
| </Text> | ||
| </Button> |
There was a problem hiding this comment.
<Text/>는 더 이상 사용되지 않습니다! 두 가지 이유가 있었어요.
<Text/>를 사용하면 css 토큰화(디자인 토큰 형식 CSS 스타일링 #326 이슈 참조)를 결정한 이유와 같이 스타일의 재사용성이 떨어졌어요.<Text/>는 DOM 트리에서 스타일(텍스트 크기, 색상)과 상관없이 p 태그로 렌더링되기 때문에, Semantic HTML의 관점에서 웹페이지의 의미(제목/본문 등)를 제대로 표현하지 못해요.
이런 이유로 기존에 가 사용된 코드들을 하나씩 바꾸고 있는데, 이번 PR에서도 바꾸어보시면 좋아요!
"boldtitle3" variant 대신 text.boldtitle3 css token을 사용하여 아래와 같이 변경할 수 있습니다.
import { w, h, text, colors } from "@/styles";
...
<Button
css={[
w(270),
h(38),
text.boldtitle3,
validated ? colors.blue600 : colors.gray300,
]}
onClick={onSubmit}
disabled={!validated}
>
투표 생성하기
</Button>
...
|
내꺼에서 저장하면 요렇게 diff가 떠 |
|
고생 많으셨어요!! 좋아보여요 :) |
|
말씀해주신대로 고쳐서 다시 push했습니다...!!!! 감사합니당>< |
|
#393 피알 먼저 고치구 메인에 머지한 다음에 |
| w(270), | ||
| h(38), | ||
| text.boldtitle3, | ||
| validated ? colors.blue600 : colors.gray300, |
There was a problem hiding this comment.
Button이 이미 자체 style을 가지고 있는 상황이니까 Button 외부에서 스타일을 주입해주는것보다 내부에 disabled style variant를 가지도록 하는게 낫지 않을까요??
There was a problem hiding this comment.
앗 그리고 colors는 css prop이 아니라 그냥 색깔값이라서 이렇게 넣어주면 아무런 효과가 없을거에요!
아마 text.blue600이나 bg.blue600을 의도하신것 같아요
| @@ -1,7 +1,7 @@ | |||
| import React, { useCallback, useMemo } from "react"; | |||
| import { css } from "@emotion/react"; | |||
|
|
|||
| import { EmoticonIcon, SendIcon } from "@/assets"; | |||
| // import { EmoticonIcon, SendIcon } from "@/assets"; | |||
| @@ -86,7 +86,7 @@ export const ChatInput: React.FC<Props> = ({ send }) => { | |||
| /> | |||
| <Divider dir="vertical" /> | |||
| </div> | |||
| <EmoticonIcon /> | |||
| {/* <EmoticonIcon /> */} | |||
|
@oganessone718 main이랑 conflict 나는 거 resolve하고 나서 pr 요청하는 게 좋아요! conflict resolve commit이 새로 생겨도 리뷰를 새로 받아야 메인에 머지가 가능해서.. 천천히 하구 다시 피알 요청해주세용 시험 화이팅 |
fixes #241