Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new Tooltip component to the design system, comprising TooltipView, TooltipOverlayView, and a TooltipFactory for management. It also refactors CharacterInputView and ToLoginView to utilize enums for better type safety and updates ToastFactory to an enum. The review feedback highlights opportunities to reduce code duplication regarding window access and hardcoded layout constants, optimize layer management in layoutSubviews, remove the unused TooltipArrowView file, and enhance the user experience by adding a fade-out animation during tooltip dismissal.
| static var window: UIWindow? { | ||
| UIApplication.shared | ||
| .connectedScenes | ||
| .flatMap { ($0 as? UIWindowScene)?.windows ?? [] } | ||
| .first { $0.isKeyWindow } | ||
| } |
There was a problem hiding this comment.
최상단 window를 찾는 로직이 ToastFactory.swift와 완전히 중복되어 구현되어 있습니다. 향후 윈도우 탐색 로직이 변경될 경우 두 곳을 모두 수정해야 하는 번거로움이 있으므로, UIWindow extension 등으로 분리하여 공통으로 사용하는 것이 좋습니다.
…nto fix/#315-Components # Conflicts: # MLS/MLSDesignSystem/Sources/MLSDesignSystem/Components/Tooltip/TooltipView.swift # MLS/MLSDesignSystem/Sources/MLSDesignSystem/Layouts/Factory/TooltipFactory.swift
dongglehada
left a comment
dongglehada
left a comment
There was a problem hiding this comment.
수고 많으셨습니다! 의견이 있거나 궁금한 부분들이 주요하지는 않은 것 같아서 승인처리 하였고, 따로 코멘트만 확인 부탁드리겠습니다 :>
| // MARK: - Properties | ||
| private let disposeBag = DisposeBag() | ||
|
|
||
| var onDismiss: (() -> Void)? |
There was a problem hiding this comment.
onDismiss 를 Rx기반 대신 클로저로 사용한 이유가 궁금합니다! 지금 import 를 보니 rx에 의존성이 이미 있는 view로 보여서요!
There was a problem hiding this comment.
기존에 rx 의존성 없이 구현중이던 코드를 그대로 이어 갔네요.. 확실하게 rxGesture로 수정하였습니다~
| import RxCocoa | ||
| import RxSwift | ||
|
|
||
| final class TooltipOverlayView: UIView { |
There was a problem hiding this comment.
모듈 내에서 internal 수준의 접근 제어가 필요한 상황에는 생략하는게 좋을지 명시적으로 표기하는게 좋을지 의견이 궁금합니다
There was a problem hiding this comment.
혼동을 피하기 위해서라도 명시적으로 붙여주는게 맞는 것 같아요!!
2 participants
📌 이슈
✅ 작업 사항
로그인 유도 UI를 진입 경로에 따라 분기 처리할 수 있도록 수정
레벨 및 직업 입력 UI를 진입 경로에 따라 분기 처리할 수 있도록 수정
도감 리스트 셀에 랭킹 태그를 추가하기 위하여 CardList 및 TagChip 컴포넌트 수정
Tooltip 표시를 위한 컴포넌트 및 Factory 구현
+추가 : 기존 class 타입이었던 ToastFactory를 enum으로 변경하여 인스턴스 생성 방지 및 안정성 개선