-
Notifications
You must be signed in to change notification settings - Fork 5
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.
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
refactor: 사전 식물 이미지 해상도를 조정한다 #387
Merged
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
하나의 url로 로딩 실패하면 다음 url을 불러오는 방식
유틸 함수가 계산 이슈로 픽셀만 받을 수 있어서 그에 맞게 변경함
로컬에서는 너무나 잘 되지만 github action에서는 임의의 확률로 실패하는 코드
github action에서만 실패하는 수상한 코드 임시 주석 처리
실험중입니다
hozzijeong
approved these changes
Sep 21, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드적으로 궁금한건 딱히 없습니다!
다만... 기능이 제대로 동작하는지 확인해보려면 develop에서 확인 해봐야 할 것 같아요! 그래서 일단 Approve하겠습니다.
동작 방식에 대한 빠른 이해 실패
리다이렉션을 확실히 하도록 하기 위함
bassyu
approved these changes
Sep 21, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
깔끔하네요~!
PR 메세지 좋아요 👍
로직들이 막힘없이 이해됐어요 👍
rawfishthelgh
pushed a commit
that referenced
this pull request
Sep 22, 2023
* refactor: 사전 식물 이미지 해상도를 조정한다 (#387) * feat: 사진 크기에 따라 url을 제시하는 유틸 함수 * feat: 확장자별로 사진 로딩 시도 하나의 url로 로딩 실패하면 다음 url을 불러오는 방식 * refactor: Image의 size 픽셀만 가능하게 변경 유틸 함수가 계산 이슈로 픽셀만 받을 수 있어서 그에 맞게 변경함 * test: 원인모를 cypress 실패 주석처리 로컬에서는 너무나 잘 되지만 github action에서는 임의의 확률로 실패하는 코드 * test: 동작이 이상한 테스트 코드 제거 github action에서만 실패하는 수상한 코드 임시 주석 처리 * chore: cypress github action 버전 업그레이드 * chore: github action 수정 실험중입니다 * test: auth redirection 토스트 확인 삭제 * chore: e2e ci 실패시 스크린샷 저장 * chore: upload artifact 버전 명시 * chore: upload artifact 삭제 동작 방식에 대한 빠른 이해 실패 * test: 접속 후 대기시간 추가 리다이렉션을 확실히 하도록 하기 위함 * test: github actions에서만 안되는 테스트 제거 * chore: e2e ci 실패 슬랙 알림 활성화 * fix: 사용자 사진 업로드 시 미리보기가 안 바뀌는 문제 (#390) * feat: url이 피움 static인지 확인하는 유틸 함수 * fix: src를 바꿔도 이미지가 변하지 않는 현상 - 맨 처음 주어진 src로 ref 안에 배열을 가두어 새로운 src가 주어져도 사진이 변하지 않음 - setErrorImage 자체는 일종의 비제어 컴포넌트처럼 동작, ref를 이용할 이유 X * refactor: useFileUpload 호출마다 새 url 생성 방지 * test: msw mock data image url 변경 피움 static 서버에서 받아오도록 수정 * refactor: 각종 리팩터링 모음 (#391) * design: line-height 설정 * refactor: 모두의 정원 margin-bottom 설정 * refactor: 위치에 따라 select박스 위치 변경 * design: line-height 설정 * refactor: pending일 경우에 button block * refactor: modal 오픈시에 scroll 방지 * refactor: select 상태 제거 * design: 모두의 정원 header 수정 * chore: v1.1.1로 업데이트 --------- Co-authored-by: 정호진 <[email protected]>
hozzijeong
pushed a commit
that referenced
this pull request
Sep 26, 2023
* feat: 사진 크기에 따라 url을 제시하는 유틸 함수 * feat: 확장자별로 사진 로딩 시도 하나의 url로 로딩 실패하면 다음 url을 불러오는 방식 * refactor: Image의 size 픽셀만 가능하게 변경 유틸 함수가 계산 이슈로 픽셀만 받을 수 있어서 그에 맞게 변경함 * test: 원인모를 cypress 실패 주석처리 로컬에서는 너무나 잘 되지만 github action에서는 임의의 확률로 실패하는 코드 * test: 동작이 이상한 테스트 코드 제거 github action에서만 실패하는 수상한 코드 임시 주석 처리 * chore: cypress github action 버전 업그레이드 * chore: github action 수정 실험중입니다 * test: auth redirection 토스트 확인 삭제 * chore: e2e ci 실패시 스크린샷 저장 * chore: upload artifact 버전 명시 * chore: upload artifact 삭제 동작 방식에 대한 빠른 이해 실패 * test: 접속 후 대기시간 추가 리다이렉션을 확실히 하도록 하기 위함 * test: github actions에서만 안되는 테스트 제거 * chore: e2e ci 실패 슬랙 알림 활성화
Choi-JJunho
pushed a commit
that referenced
this pull request
Oct 20, 2023
* feat: 사진 크기에 따라 url을 제시하는 유틸 함수 * feat: 확장자별로 사진 로딩 시도 하나의 url로 로딩 실패하면 다음 url을 불러오는 방식 * refactor: Image의 size 픽셀만 가능하게 변경 유틸 함수가 계산 이슈로 픽셀만 받을 수 있어서 그에 맞게 변경함 * test: 원인모를 cypress 실패 주석처리 로컬에서는 너무나 잘 되지만 github action에서는 임의의 확률로 실패하는 코드 * test: 동작이 이상한 테스트 코드 제거 github action에서만 실패하는 수상한 코드 임시 주석 처리 * chore: cypress github action 버전 업그레이드 * chore: github action 수정 실험중입니다 * test: auth redirection 토스트 확인 삭제 * chore: e2e ci 실패시 스크린샷 저장 * chore: upload artifact 버전 명시 * chore: upload artifact 삭제 동작 방식에 대한 빠른 이해 실패 * test: 접속 후 대기시간 추가 리다이렉션을 확실히 하도록 하기 위함 * test: github actions에서만 안되는 테스트 제거 * chore: e2e ci 실패 슬랙 알림 활성화
hozzijeong
pushed a commit
that referenced
this pull request
Oct 20, 2023
* feat: 사진 크기에 따라 url을 제시하는 유틸 함수 * feat: 확장자별로 사진 로딩 시도 하나의 url로 로딩 실패하면 다음 url을 불러오는 방식 * refactor: Image의 size 픽셀만 가능하게 변경 유틸 함수가 계산 이슈로 픽셀만 받을 수 있어서 그에 맞게 변경함 * test: 원인모를 cypress 실패 주석처리 로컬에서는 너무나 잘 되지만 github action에서는 임의의 확률로 실패하는 코드 * test: 동작이 이상한 테스트 코드 제거 github action에서만 실패하는 수상한 코드 임시 주석 처리 * chore: cypress github action 버전 업그레이드 * chore: github action 수정 실험중입니다 * test: auth redirection 토스트 확인 삭제 * chore: e2e ci 실패시 스크린샷 저장 * chore: upload artifact 버전 명시 * chore: upload artifact 삭제 동작 방식에 대한 빠른 이해 실패 * test: 접속 후 대기시간 추가 리다이렉션을 확실히 하도록 하기 위함 * test: github actions에서만 안되는 테스트 제거 * chore: e2e ci 실패 슬랙 알림 활성화
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.
🔥 연관 이슈
🚀 작업 내용
사진 변환 코드 (별도의 폴더에서 따로 진행)
💬 리뷰 중점사항
로직이 좀 특이합니다.
picture / source를 안 쓴 이유
source에 webp가 들어있다고 칩시다. webp 브라우저를 지원하는 곳에서는 webp 주소로 로딩을 시도합니다.
근데 만약 저희가 S3에 해당 이미지의 webp 버전을 제공하지 않으면? 터집니다. 근데 여기서
이런 이지선다에 걸렸어요
제가 원하는 동작 방식은 webp 있는지 -> 없으면 png 있는지 -> 없으면 원본 -> 진짜없으면 피우미 이거였는데 source의 동작 방식은 이거랑 완전히 다르더라구요. source의 핵심은 '이게 있는가?'가 아니라 '이 확장자를 지원하는가?'여서 관심사 차이가 심해 사용하지 않는 게 오히려 구현이 편했습니다.
그래서 현재 로직은
이런 방식으로 구성을 해 놓았어요
장점은 webp를 지원하는 브라우저이고, s3에서 지원한다면 확실하게 처리가 가능합니다
단점은 s3에 없다면 webp -> png -> 원본 순서로 총 세 번의 요청을 보내야 해서 이게 과연 맞는 방법인지는 모르겠습니다
(unsplash에서 이미지를 받아오면 세 번의 요청을 해야 하는데 육안으로는 큰 차이를 느끼진 못했어요)