Skip to content
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

[SKP-141-image api] 이미지 분석, 수정 API 연결 #30

Merged
merged 12 commits into from
Aug 21, 2024

Conversation

yusiny
Copy link
Member

@yusiny yusiny commented Aug 21, 2024

🪄 Issue Number

🏆 Details

  • 분석 API 수정 반영
  • 재분석 API 연결
  • 카테고리 수정 API 연결

✅ Need Review

  • 공유하기로 넘어왔을 때, 분석 API 호출하는 부분 TODO
  • 다시 분석하기 누르면 팝업 하나 정도는 떠야 할 것 같은데 ,, 무거운 작업인데 바로 실행되는 거 같아서요 흠

📸 Screenshot

1. 이미지 분석

전체 성공 부분 실패 전체 실패
1.mp4
default.MP4
default.mov

2. 재분석, 카테고리 수정

카테고리 수정 재분석
default.MP4
default.MP4

@yusiny yusiny added the feat💡 기능 추가 label Aug 21, 2024
@yusiny yusiny self-assigned this Aug 21, 2024
@yusiny yusiny requested a review from imeureka as a code owner August 21, 2024 07:38
Copy link
Collaborator

@imeureka imeureka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

깔끔하고 영상보니 다 부드럽게 반영되고 있네요 너무 좋습니다.

);
}

const styles = StyleSheet.create({
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

분리 부탁함다

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

앗 수정했습니다 ㅎㅎ

Comment on lines +116 to +118
navigation.replace('ReAnalyze', {history: result, request: request});
console.log(request);
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

navigate 대신 replace 사용하신 이유가 궁금합니다.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

navigate로 하면 스택에 쌓여서, 기존 화면을 교체하려고 replace 사용했어요
AnalyzeResult -> ReAnalyze -> AnalyzeResult
이렇게 이동하는데 기존 화면 남아있으면 안되니까!

Comment on lines +39 to +42
queryClient.invalidateQueries({
queryKey: LOCATION_KEYS.detail(String(id)),
});
bottomSheetRef.current?.close();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오 너무 좋아요 그래서 반영이 빨랐던 건가

Copy link
Member Author

@yusiny yusiny Aug 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

음 refetch랑 invalidateQueries에 속도 차이가 있나요?
저는 아래 내용으로 알고 있어서, 무효화시킬 해당 쿼리 키만 무효화 시켜서 active한 화면 (여기서는 Detail이겠죠?) 에서 데이터를 새로 불러오도록 작업한 거거든요
active한 페이지만 새로 불러와서 속도가 빨랐다고 느끼는 거려나 ,,

refetch는 모든 페이지의 데이터를 불러오지만 (키를 기준으로 inactive 한 데이터 까지 불러옴)
invalidation은 우선 query를 stale하게만 만들기 때문에 active한 페이지의 데이터를 가져옴
참고

Copy link
Collaborator

@imeureka imeureka Aug 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오호 저도 한번 공부해보겠습니다 refetch랑 invalidateQueries 차이가 아닐수도 있겠네요
active한 페이지만 새로 불러와서 속도가 빨랐다고 느끼는 거려나 ,,
ㄴ 그런 듯요 ㅎㅎ

@@ -4,11 +4,15 @@ import useNavigator from '../../navigators/hooks/useNavigator';
export default function MoreSettingScreen() {
const {stackNavigation} = useNavigator();
function handle() {
stackNavigation.navigate('Detail', {id: 79});
stackNavigation.navigate('Detail', {id: 470});
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 박혀져 있는 아이디값은 뭔가요?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

앗 이거 ㅎㅎ 상세 화면 테스트용으로 id 박아둔건데, 더보기 화면 작업하시면서 다 지우셔도 돼요~

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아아!!! 감사함니당

}, [request]);

return (
<SafeAreaView style={styles.container}>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

요기서 View 대신 SafeAreaView 쓰신 이유가 있나요? App에서 상단에 SafeAreaView로 감싸긴 했는데 혹시 오류가 있었나여???

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

App.tsx에는 <SafeAreaProvider/>로 감싸져 있고, 각 화면에서 <SafeAreaView/>로 감싸서 사용하는 걸로 알고 있는데, 아닌가요?

@yusiny yusiny merged commit 345c757 into main Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat💡 기능 추가
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[SKP-141] [FRONT] 분석, 수정 API 연결
2 participants