-
Notifications
You must be signed in to change notification settings - Fork 0
[Feat] 제휴 정보 토글 default상태 변경 및 Map 관련 코드 리팩토링 #417
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
Merged
Merged
Changes from 23 commits
Commits
Show all changes
24 commits
Select commit
Hold shift + click to select a range
6ff95c0
feat: 제휴지도 학과/전체 토글 디폴트값 변경 및 토글 UI 위치 변경
kangyuri1114 f1d7e6f
feat: PartnershipFilterToggle에서 toggleItem 생성방식 변경 및 UI수정, 불필요 코드 삭제
kangyuri1114 a8b14fb
delete: 사용하지 않는 위치 권한 코드 삭제
kangyuri1114 b54287c
feat: compose 라이브러리 버전 업데이트
kangyuri1114 30eaefc
feat: compose router-screen 분리
kangyuri1114 f57a713
feat: 학과 선택을 하지 않은 경우, default 토글이 "전체"로 수정. 학과 토글 선택 시 바텀시트 보여주기
kangyuri1114 346e0ed
feat: 제휴지도 학과/전체 토글 디폴트값 변경 및 토글 UI 위치 변경
kangyuri1114 8c5adb9
feat: PartnershipFilterToggle에서 toggleItem 생성방식 변경 및 UI수정, 불필요 코드 삭제
kangyuri1114 40d912c
delete: 사용하지 않는 위치 권한 코드 삭제
kangyuri1114 23bfca2
feat: compose 라이브러리 버전 업데이트
kangyuri1114 c3884e0
feat: compose router-screen 분리
kangyuri1114 037cef4
feat: 학과 선택을 하지 않은 경우, default 토글이 "전체"로 수정. 학과 토글 선택 시 바텀시트 보여주기
kangyuri1114 5e2ffc9
Merge remote-tracking branch 'origin/feat/replace-map-toggle-default'…
kangyuri1114 6ae0eac
feat: compose 버전 업데이트 롤백
kangyuri1114 d8e2c78
feat: 학과 정보가 업데이트될 때마다 토글 상태 없데이트하도록 key 변경
kangyuri1114 77487ef
feat: init 내부에서 전체 제휴정보 load하는 코드 제거(compose LaunchedEffect로 이동)
kangyuri1114 c2006c4
feat: BottomSheet 표시는 View의 SheetState로만 관리 (ViewModel은 데이터만 제공), 제휴정…
kangyuri1114 be080a7
feat: Domain 모델(RestaurantType)을 UI 모델(PlaceType)로 변환 로직을 뷰모델로 이동
kangyuri1114 7bf29b2
feat: 토글 필터 상태, 이벤트 로거를 ViewModel로 이동. departmentId, collegeId flow 방…
kangyuri1114 84bbc3d
[Hotfix] Release에서 발생하던 문제 해결 및 3.1.8 릴리즈 (#418)
PeraSite 10036e9
chore: material 의존성 제거 (#423)
HI-JIN2 8a974e2
feat: 코드 리뷰 반영 (네이밍 변경 및 scope 전달 -> 람다 전달로 수정, 최초 정보 load 시 state co…
kangyuri1114 052b826
feat: 제휴정보 토글 변경 시 선택했던 식당의 제휴정보 state 초기화
kangyuri1114 7d7731c
feat: MapScreen 접근 제어자 private -> internal 변경
kangyuri1114 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
Oops, something went wrong.
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.
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.
p2 저는 internal fun으로 하는 편입니다
요즘 요렇게 많이 쓰이는 것 같아요. 비슷한 접근제어자라서 제 견해로는 UI 전체인지, 컴포넌트인지를 구분하는 용도로 써도 좋을 것 같아요
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.
저는 MapScreen / internal fun MapScreen 이렇게 하는 편인데,
전에 유리님께서 route 네이밍을 얘기하셨어서, MapRoute / internal MapScreen 요렇게 절충안을 제안해봅니다!
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.
internal은 모듈 내에서 접근 가능, private은 파일 내 접근 가능으로 알고 있었습니다
리뷰를 읽고 사실 굳이 internal을 써야 할까 의문이 들었었는데,
곰곰쓰 생각해보니까
하나의 compose 파일 내에 모든 screen이 존재하라는 보장이 없으니(길어지면 보통 분리하니까! 또는 Screen이 여러개라면 별도 파일로 둘 수도 있고) private보다는 internal이 맞겠네요!
내부 컴포넌트는 core컴포넌트가 아닌 이상 보통 한 파일에 두니까 private이 맞을 것 같구요
이부분도 wiki에 적어두면 좋을 거 같아요
그런데 이렇게 정한 이유가 위에 제가 설명한 내용이 맞는지 검토해주시고 동의해주시면 wiki에 적어두겠습니다!
우선 해당 부분 코드는 수정했습니다!
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.
맞습니다~
근데 또 생각해보니까 내부컴포넌트도 별도의 파일로 분리하고 있지 않나? 라는 생각이 듭니다 🤔