-
Notifications
You must be signed in to change notification settings - Fork 0
[Fix] 최종 QA 반영 #218
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
[Fix] 최종 QA 반영 #218
Conversation
#Conflicts: # Wable-iOS/Presentation/Home/View/HomeDetailViewController.swift # Wable-iOS/Presentation/Login/LoginViewController.swift
|
""" WalkthroughThe changes update auto-login logic in the app launch flow to be synchronous, refine keyboard dismissal and comment posting UX in the home detail view, enforce minimum height and hide the tab bar in the write post view, and adjust the logic for checking empty nicknames in the login view controller. Changes
Sequence Diagram(s)sequenceDiagram
participant SceneDelegate
participant UserSessionRepository
participant MainScreen
participant LoginScreen
SceneDelegate->>UserSessionRepository: Fetch active session
alt Session exists and auto-login enabled and nickname not empty
SceneDelegate->>MainScreen: Configure main screen
else
SceneDelegate->>LoginScreen: Configure login screen
end
sequenceDiagram
participant HomeDetailVC
participant User
participant Toast
participant CollectionView
User->>HomeDetailVC: Tap createCommentButton
HomeDetailVC->>HomeDetailVC: Disable button interaction
HomeDetailVC->>HomeDetailVC: Post comment
alt Success
HomeDetailVC->>Toast: Show success toast
HomeDetailVC->>CollectionView: Scroll to top
HomeDetailVC->>HomeDetailVC: Clear text view, update placeholder, enable button
else
HomeDetailVC->>HomeDetailVC: Enable button
end
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
Wable-iOS/App/SceneDelegate.swift(1 hunks)Wable-iOS/Presentation/Home/View/HomeDetailViewController.swift(4 hunks)Wable-iOS/Presentation/Home/View/WritePostViewController.swift(2 hunks)Wable-iOS/Presentation/Login/LoginViewController.swift(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
Wable-iOS/App/SceneDelegate.swift (1)
Wable-iOS/Data/RepositoryImpl/UserSessionRepositoryImpl.swift (1)
fetchActiveUserSession(39-44)
🪛 SwiftLint (0.57.0)
Wable-iOS/App/SceneDelegate.swift
[Warning] 128-128: Using ternary to call Void functions should be avoided
(void_function_in_ternary)
🔇 Additional comments (8)
Wable-iOS/App/SceneDelegate.swift (1)
121-126: LGTM! Good defensive programming with early return pattern.The synchronous approach using
fetchActiveUserSession()simplifies the logic compared to the previous asynchronous Combine chain. The guard statement with early return properly handles cases where session or auto-login flag is unavailable.Wable-iOS/Presentation/Login/LoginViewController.swift (1)
157-157: LGTM! Consistent empty string checking.The explicit comparison
== ""aligns with the similar change inSceneDelegate.swift(line 128), ensuring consistent nickname validation across the app launch and login flows.Wable-iOS/Presentation/Home/View/WritePostViewController.swift (2)
87-87: LGTM! Proper navigation behavior.Setting
hidesBottomBarWhenPushed = trueensures the tab bar is hidden when this view controller is pushed, providing a cleaner focused writing experience.
142-144: LGTM! Ensures adequate text input space.The minimum height constraint of 300 points provides sufficient space for content input, improving the writing experience by preventing the text view from being too small.
Wable-iOS/Presentation/Home/View/HomeDetailViewController.swift (4)
197-197: LGTM! Proper gesture handling setup.Adding the tap gesture delegate enables fine-grained control over gesture recognition, which is used in the UIGestureRecognizerDelegate implementation to prevent keyboard dismissal when tapping the comment button.
754-760: LGTM! Smart gesture filtering.The delegate implementation correctly prevents the tap gesture from dismissing the keyboard when the user taps the
createCommentButton, improving the commenting UX by avoiding unintended keyboard dismissals.
582-582: LGTM! Prevents multiple submissions.Disabling user interaction immediately when the comment button is tapped prevents multiple rapid submissions while the comment is being processed, which is a good UX practice.
681-684: LGTM! Improved operation sequencing.The reordering of operations (showing toast and scrolling before resetting the text view) provides better visual feedback to the user. Re-enabling button interaction and updating placeholder state after successful posting ensures proper UI state management.
Also applies to: 691-692
| isAutologinEnabled ? owner.configureMainScreen() : owner.configureLoginScreen() | ||
| } | ||
| .store(in: cancelBag) | ||
| (session.nickname != "" && isAutoLoginEnabled) ? configureMainScreen() : configureLoginScreen() |
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.
🛠️ Refactor suggestion
Fix the ternary operator usage with void functions.
SwiftLint correctly flags this pattern as it can reduce readability when calling void functions.
Apply this diff to improve readability:
-(session.nickname != "" && isAutoLoginEnabled) ? configureMainScreen() : configureLoginScreen()
+if session.nickname != "" && isAutoLoginEnabled {
+ configureMainScreen()
+} else {
+ configureLoginScreen()
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| (session.nickname != "" && isAutoLoginEnabled) ? configureMainScreen() : configureLoginScreen() | |
| if session.nickname != "" && isAutoLoginEnabled { | |
| configureMainScreen() | |
| } else { | |
| configureLoginScreen() | |
| } |
🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 128-128: Using ternary to call Void functions should be avoided
(void_function_in_ternary)
🤖 Prompt for AI Agents
In Wable-iOS/App/SceneDelegate.swift at line 128, the ternary operator is used
to call void functions, which reduces readability and is flagged by SwiftLint.
Replace the ternary operator with a clear if-else statement that calls
configureMainScreen() if session.nickname is not empty and isAutoLoginEnabled is
true; otherwise, call configureLoginScreen(). This improves code clarity and
adheres to SwiftLint guidelines.
JinUng41
left a comment
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.
고생하셨습니다~ 👍👍🏻👍🏼👍🏽👍🏾👍🏿
| .sink { owner, isSucceed in | ||
| if isSucceed { |
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.
filter를 사용해도 좋을 것 같습니다.
👻 PULL REQUEST
📄 작업 내용
💻 주요 코드 설명
앱 진입 시 회원가입 여부 판별 로직 추가
👀 기타 더 이야기해볼 점
🔗 연결된 이슈
Summary by CodeRabbit
New Features
Bug Fixes
Improvements