Avatar, Status 컴포넌트 추가 #314
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sungik-choi
force-pushed
the
features/avatar
branch
from
2af3d00 to
94e8ec4
Compare
Closed
Closed
sungik-choi
commented
Apr 12, 2021
sungik-choi
commented
Apr 12, 2021
sungik-choi
requested review from
jwoo0122,
danivelop and
quino0627
and removed request for
jwoo0122
sungik-choi
force-pushed
the
features/avatar
branch
from
57a7bd6 to
645c67d
Compare
sungik-choi
commented
Apr 15, 2021
| if (enableSmoothCorners.current) { return '' } | ||
| return css` | ||
| background-color: ${({ foundation }) => foundation?.theme?.['bg-grey-light']}; | ||
| background-image: ${`url(${avatarUrl})`}; | ||
| background-image: ${`url(${avatarUrl}), url(${fallbackUrl})`}; |
There was a problem hiding this comment.
이렇게 background-image를 겹치는 방식으로 사용 시, 투명도가 있는 이미지를 사용한 사용자들의 프로필 뒤에 이미지가 보이는 문제가 있어서 기존에 작성해두었던 hook을 사용하는 방식으로 변경했습니다.
sungik-choi
commented
Apr 15, 2021
jwoo0122
suggested changes
Apr 15, 2021
sungik-choi
force-pushed
the
features/avatar
branch
from
e778a7d to
d0174e6
Compare
- 빠진 인터페이스 속성 추가 및 수정
jwoo0122
reviewed
Apr 19, 2021
jwoo0122
reviewed
Apr 19, 2021
This reverts commit 6e26a5a.
quino0627
reviewed
Apr 20, 2021
|
Check 기능 대기 중입니다. 별도의 PR 로 작성하고 싶으신 경우에는 말씀해주세요. |
Avatar 베이스로 해서 별도 PR로 작성하려고 합니다! 미리 말씀을 못드렸네요 |
- 함수 내에서 uuid를 생성하고 부여하기 때문에, render시 key가 불필요하게 새로 생성되고, key를 기반으로 변화를 감지하기 힘들게 됨 - avatarUrl + name을 조합해서 사용하면 충분히 key의 고유성을 가질 수 있다고 판단 - name에 빈 스트링이 들어가지 않게 하기 위해 optional 속성이었던 name을 필수 속성으로 변경
jwoo0122
approved these changes
Apr 20, 2021
groovypark
approved these changes
Apr 21, 2021
inhibitor1217
approved these changes
Apr 21, 2021
psi9730
pushed a commit
to psi9730/bezier-react
that referenced
this pull request
May 25, 2021
* Avatar 컴포넌트 기본 구조 작성 (테스트 코드, 스토리북 제외) * Avatar 스토리북 생성 * smoothCorner shadow props 기본값 변경 * 아바타 곡률 변경 * smooth corner를 border-radius와 비교하기 위해 스토리북 업데이트 * 스타일 순서 lint 에러 수정 및 불필요한 템플릿 문자열 제거 * 아바타 인터페이스 고도화 및 XXXS 사이즈 추가 * AvatarGroup 컴포넌트 추가 * 스토리북에 AvatarGroup 추가 * 스타일 린트 에러 수정 및 상수 분리 * 컨벤션 및 변수 수정 * showBorder 기능 추가 및 변수명 수정 - 평소엔 보더가 보이지 않고, spacing을 음수로 설정했을 경우에만 보더가 드러나도록 구현 - Ellipsis 아바타의 경우, 아바타가 아닌 EllipsisWrapper가 마진을 갖도록 변경. 아이콘의 위치 배치를 용이하게 하기 위함. * 아바타 그룹 testId 추가 * disabled 타입 구현 * smoothCorner 미지원 브라우저 스타일 fallback 처리 * key 추가 및 조건문 리팩토링 * 잘못된 데이터 속성 key 값 수정 * 이모지에서만 사용하는 XXXS 사이즈 삭제 - 나중에 이모지가 아바타로 포함된다면 그 때 추가할 예정. 지금은 필요없음 * Status 컴포넌트 추가 및 아바타에 status 관련 속성 추가 * 스토리북 업데이트 * Status 컴포넌트를 별도 파일로 분리, 스타일 분리 * Icon 잘못된 Import 순서 변경 * Avatar와 AvatarComponent 분리 * 스토리북 파싱 에러 해결을 위해 폴더명 Avatar -> Avatars 로 변경 * Avatar, AvatarGroup 개별 스토리북 업데이트 * 컴포넌트 export 추가 * 스토리북 변수명 변경 및 변수 분리 * 린트 warning 해결을 위한 구조분해 * 스토리북 오류로 사용처에서 변수 선언해서 사용하도록 변경 * Smooth Corner 미지원 브라우저에서 Border fallback 처리 추가 - css를 의미있는 변수로 리팩토링 - 상수 컨벤션 수정 - 주석 추가/수정 * AvatarGroup에서 avatar에 showBorder 속성이 true인 경우, 오버라이드 하지 않도록 변경 * css 스타일 변수명 컨벤션 통일 * fallback css에 smoothCorner 지원 여부 조건 추가 * smoothcorner -> smoothcorners 로 변경 - 주석 띄어쓰기 변경 * 잘못된 스타일 함수 리턴값 수정 * smoothCorner 미지원 브라우저 disable 스타일 에러 해결 - smoothCorner 스타일 코드에 아래에 있는 코드는 무시돼서 상위로 끌어올림 * 기본 아바타 fallback 처리 로직 구현 - useProgressiveImage 커스텀 훅으로 이미지가 로드되었을 시에만 소스를 리턴하도록 수정 - svg 코드를 data url로 변환하여 기본값으로 지정 - https://gist.github.com/iansinnott/2e8fe9d9e4c6c7c55793d38f81c999a3 * DefaultAvatar를 아바타 컴포넌트 내에서 변환하지 않도록 수정 * @types/react-dom 설치 * svg to data url 변환 로직 유틸함수로 이동 * 불필요한 StatusWrapper 렌더링 제거 * Status 스토리북 추가 * Avatar 잘못된 인터페이스 변경 * 주석 변경 및 제거 * Avatar, AvatarGroup 스토리북 타입 강화 * Avatar Custom Badge 스토리북 case 추가 및 children 렌더 로직 변경 * Status NONE 타입 삭제 및 관련 로직, 스토리북 변경 * lodash to lodash-es * border가 쌓임 맥락이 제대로 형성되도록 수정 - http://www.independent-software.com/set-stacking-order-of-pseudo-elements-below-parent-element.html - AvatarGroup의 CSS Selector를 Owl Selector로 변경 * disabled(opacity) 속성이 올바르게 적용되도록 수정 - 올바른 쌓임 맥락(부모 엘리먼트)에서 opacity를 적용하기 위해 Wrapper에서 disabled를 적용하도록 변경. 기존처럼 Avatar에서 disabled를 적용하면 새로운 쌓임 맥락이 생성되고, 기존에 적용되었던 쌓임 맥락이 망가지게 됨. * disabled 속성이 다른 속성에 영향을 주지 않도록 변경 * 불필요한 마진 계산 제거 * useProgressiveImage에서 이미지를 캐시하도록 변경 - 기존에 useState 초기값을 null로 가져갔기 때문에, 브라우저에서 이미지를 캐시하더라도 다시 이미지가 마운트될때는 기본 이미지가 보였다(깜빡이는듯한 현상 발생). 이를 방지하기위해 내부에 이미지 캐시를 만들고, 마운트될 때 state 초기값을 캐시에 접근해서 가져올 수 있도록 했다. * default 이미지를 커스텀 훅에서 리턴하도록 변경 * 이미지를 캐시하는 대신 smoothCorners에서 fallback 이미지를 그려주도록 변경 기존에 캐시를 사용하려고 했던 이유는 smooth-corner에서 background-image를 사용할 수 없기 때문이었습니다. background-image에서는 2개이상의 url을 넣으면 쌓임 맥락에 의해 뒤의 이미지가 아래에 쌓이고, 원본 이미지가 로드되지 않을 경우 자연스럽게 보이게 됩니다. fallback 이미지로 쓰이는 이미지의 종류는 아바타에서 하나이고, 브라우저에서 이미 자체적으로 캐시를 해주고 있기 때문에 이미지 2개를 그리는 오버헤드는 크지 않을 거라 판단했지만, 앞에서 말했듯 smooth-corner를 사용하면 background paint를 사용하기에 사용이 불가능했습니다. CSSImageValue를 사용하기 위해 선택지 중 하나인 list-style-image를 통해 fallback 이미지를 넘겨주었고, smoothCorners 스크립트에서 원본 이미지에 까는 방식으로 해결했습니다. * 기본 아바타 색상 업데이트 * 잘못된 mixin 수정 * Revert "잘못된 mixin 수정" This reverts commit cba8f25. * Revert "이미지를 캐시하는 대신 smoothCorners에서 fallback 이미지를 그려주도록 변경" This reverts commit 56ffc85. * 아바타 기본 색상 변경 * StatusType enum 파스칼케이스로 변경 * AvatarGroup Ellipsis시 Count 아바타 추가 - enum 파스칼 케이스로 변경 - import 순서 변경 * useProgressiveImage의 상수를 Enum으로 변경 * 아바타 그룹의 최대 카운트 수를 99로 변경 * 아바타 그룹 Ellipsis에 mouseEnter, mouseLeave 속성 추가 - height -> size로 변경해서 너비, 높이 모두 변경되도록 수정 * 레오 리뷰 반영: Enum 컨벤션 맞추기 * 레오 리뷰 반영: smooth Corners fallback 내용을 믹스인 내부로 이동 * 레오 리뷰 반영: 아바타 사이즈 enum 키를 영문 대문자에서 Size{숫자}로 변경 * svg 파일을 React Element로 변환하지 않고 사용하도록 변경 * 레오 리뷰 반영: key를 uuid에서 일반 string으로 변경 * svg Import 타입 에러 나지 않도록 모듈 정의 파일 추가 * avatar -> avatarUrl 로 변수명 변경 * 기본 아바타 색상을 실제 색상과 같이 변경 * Revert Install @types/react-dom * 외부 스타일 주입을 위한 Interpolation 속성 추가 - 빠진 인터페이스 속성 추가 및 수정 * svg 파일 import를 위한 @rollup/plugin-url 패키지 설치 * 레오 리뷰 반영: 삼항 연산자를 && 연산자로 변경 * 레오 리뷰 반영: 아바타 fallbackUrl을 외부에서 주입받을 수 있도록 속성 추가 * Revert "Revert Install @types/react-dom" This reverts commit 6e26a5a. * 투명 png 아바타의 경우를 고려해서 기본 배경색을 흰색으로 변경 * 삼항연산자를 && 연산자로 변경 * 레오 리뷰 반영: 스토리북 주석 추가 * 레오 리뷰 반영: 타입 선언 파일 디렉토리 변경 * 불필요한 import 제거 * 몽 리뷰 반영: 조건식 리팩토링 * 몽 리뷰 반영: 상수 분리 및 불필요한 상수 제거 * AvatarGroup의 key를 uuid가 아닌 avatarUrl과 name의 조합으로 지정 - 함수 내에서 uuid를 생성하고 부여하기 때문에, render시 key가 불필요하게 새로 생성되고, key를 기반으로 변화를 감지하기 힘들게 됨 - avatarUrl + name을 조합해서 사용하면 충분히 key의 고유성을 가질 수 있다고 판단 - name에 빈 스트링이 들어가지 않게 하기 위해 optional 속성이었던 name을 필수 속성으로 변경 * 두기 리뷰 반영: interpolation을 명확한 네이밍으로, 컨벤션 변경 * 두기 리뷰 반영: foundation.border.getBorder()로 border 스타일링
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Avatar
Status와 함께 사용시 다소 이상하게 보일텐데, 맞습니다. 큰 아이콘 사이즈에 사용할Status사이즈가 없기 때문인데, 문제가 될 거 같진 않습니다. Online/Offline Status를 큰 아바타에서 사용할 일은 (지금 프로덕트에선) 없어보입니다.AvatarGroup
Status
TODO
+초과 Avatar 수가 붙는 경우 제작 필요Changes Detail
useProgressiveImage커스텀 훅 작성Tests
Browser Compatibility
OS / Engine 호환성을 반드시 확인해주세요.
Windows
macOS
(Option) Issues
(연관된 PR이나 Task / 특정 시점까지 머지하면 안됨 / 번역 추가 필요 / ...)
Avatar의 children으로 showBorder 옵션이 켜진 Avatar를 넣고, 부모 Avatar의 showBorder 옵션을 켜면 Safari (or Firefox) 에서 제대로 보이지 않는 현상이 발생합니다.-> 해결 완료