[Feat] 프로필 수정 기능 및 키보드 표시/숨김 처리에 따른 뷰 핸들링 익스텐션 구현 #235
Conversation
WalkthroughThis update restructures the iOS project to support team selection and editing in the user profile. It introduces new UI components ( Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ProfileEditViewController
participant ProfileEditView
participant TeamCollectionView
participant NavigationView
User->>ProfileEditViewController: Navigates to profile edit
ProfileEditViewController->>ProfileEditView: Initializes with team selection closure
ProfileEditView->>TeamCollectionView: Displays teams
User->>TeamCollectionView: Taps team cell
TeamCollectionView->>ProfileEditView: Calls selection handler
ProfileEditView->>ProfileEditViewController: Updates selected team
User->>NavigationView: Taps "done" button
NavigationView->>ProfileEditViewController: Triggers doneButtonDidTap
ProfileEditViewController->>ProfileEditViewController: Checks for changes and updates profile
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Poem
✨ 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🔭 Outside diff range comments (1)
Wable-iOS/Presentation/Profile/Edit/ProfileEditViewController.swift (1)
228-239:updateDoneButtonStateignores team changes.
shouldEnableonly looks at nickname/image flags. If the user only changes the team, the Done button stays disabled, blocking the action even thoughdoneButtonDidTapsupports it.
IncludehasTeamChanged(same calc as indoneButtonDidTap) and call this method from the team-selection closure.- let shouldEnable = hasNicknameChanged || hasImageChanged + let hasTeamChanged = lckTeam != (profile.user.fanTeam?.rawValue ?? "LCK") + let shouldEnable = hasNicknameChanged || hasImageChanged || hasTeamChanged
🧹 Nitpick comments (7)
Wable-iOS/Presentation/WableComponent/Navigation/NavigationView.swift (2)
154-155: Subview ordering: keep interactive controls above decorative views
doneButtonis appended just beforemenuButton, after all other views have been added.
Although hidden views shouldn’t block touches, keeping active controls last (highest z-order) avoids accidental touch absorption if a view becomes visible due to a bug.- doneButton, - menuButton + menuButton, // decorative / less frequently tapped + doneButton // primary action – keep on top
213-218: Auto-layout: fixed size may clip on accessibility text sizesWidth = 53 pt & height = 33 pt is fine for default Dynamic Type but will truncate when the user enables larger accessibility sizes. The primary CTA should expand with its intrinsicContentSize.
- $0.adjustedWidthEqualTo(53) - $0.adjustedHeightEqualTo(33) + // Let intrinsic size drive width; pin minimum height for tap target. + $0.height.greaterThanOrEqualTo(44) // Apple HIG minimumWable-iOS/Presentation/Helper/Extension/UIViewController+KeyboardHandling.swift (2)
78-91: Shifting the whole window can cause side-effects.Animating
window.transformmoves system alerts, status bar, modals, etc. Consider translating only the VC’s view or adjusting additional-safe-area insets to avoid layout glitches and interaction issues in other windows.
81-83: Bit-twiddling animationCurve is fragile.
UIView.AnimationOptions(rawValue: curve << 16)relies on undocumented bit mapping. UseUIView.AnimationOptions(rawValue: UInt(animationCurve << 16))and guard against invalid curves, or build options viaUIView.AnimationCurve→UIView.animate(withDuration:delay:options:curve:animations:)introduced in iOS 17 for clarity.Wable-iOS/Presentation/WableComponent/View/TeamCollectionView.swift (2)
58-86:cellForItemcalls before cells exist → selection UI may fail.
selectInitialTeamqueries cells immediately; if invoked before the first layout pass,cellForItem(at:)returnsnil, so borders/text colours aren’t updated.
Dispatch tolayoutIfNeeded()first or move the styling intocellForItembased onselectedTeam.
96-102: Resetting onlyvisibleCellsmisses off-screen items.When a cell scrolls back on-screen it may keep the old selected style.
Track selection inselectedTeamand style every cell incellForIteminstead of looping throughvisibleCells.Wable-iOS/Presentation/Profile/Edit/ProfileEditViewController.swift (1)
29-32: Redundant= nilinitialisations.
sessionProfileanddefaultImageare optionals alreadynilby default. Dropping the explicit= nilremoves SwiftLint warnings.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
Wable-iOS.xcodeproj/project.pbxproj(8 hunks)Wable-iOS/Presentation/Helper/Extension/UIViewController+KeyboardHandling.swift(1 hunks)Wable-iOS/Presentation/Onboarding/View/LCKTeamView.swift(0 hunks)Wable-iOS/Presentation/Onboarding/ViewController/LCKTeamViewController.swift(2 hunks)Wable-iOS/Presentation/Profile/Edit/ProfileEditView.swift(1 hunks)Wable-iOS/Presentation/Profile/Edit/ProfileEditViewController.swift(9 hunks)Wable-iOS/Presentation/WableComponent/Navigation/NavigationView.swift(5 hunks)Wable-iOS/Presentation/WableComponent/View/TeamCollectionView.swift(1 hunks)
💤 Files with no reviewable changes (1)
- Wable-iOS/Presentation/Onboarding/View/LCKTeamView.swift
🧰 Additional context used
🪛 SwiftLint (0.57.0)
Wable-iOS/Presentation/Helper/Extension/UIViewController+KeyboardHandling.swift
[Warning] 33-33: An object should only remove itself as an observer in deinit
(notification_center_detachment)
Wable-iOS/Presentation/Profile/Edit/ProfileEditViewController.swift
[Warning] 30-30: Initializing an optional variable with nil is redundant
(redundant_optional_initialization)
[Warning] 31-31: Initializing an optional variable with nil is redundant
(redundant_optional_initialization)
🔇 Additional comments (8)
Wable-iOS/Presentation/WableComponent/Navigation/NavigationView.swift (2)
267-273: UI consistency: consider hidingpageUnderLineViewduring edit modeIn edit flows the underline may read as an inactive separator and diverge from the native edit-screen navigation pattern used elsewhere (
Settings,Contacts, etc.). Confirm with design if it should be hidden.
37-38: To ensure we catch every pattern match onPageType, let’s broaden our search beyond just"switch .*\.PageType":#!/bin/bash # List every use of the PageType identifier rg --line-number 'PageType' -g '*.swift' # Specifically look for switch statements on PageType rg --line-number 'switch .*PageType' -g '*.swift' # Look for if-case patterns matching PageType rg --line-number 'if case .*PageType' -g '*.swift'Wable-iOS.xcodeproj/project.pbxproj (6)
21-23: PBXBuildFile references for new Swift files added
Entries for ProfileEditView.swift, TeamCollectionView.swift, and UIViewController+KeyboardHandling.swift correctly inserted.
400-402: PBXFileReference entries for new source files
New file references for ProfileEditView.swift, TeamCollectionView.swift, and UIViewController+KeyboardHandling.swift are correctly configured.
1423-1423: Group association for TeamCollectionView.swift in WableComponent/View
TeamCollectionView.swift is appropriately added to the View group under WableComponent.
1788-1788: Group placement of ProfileEditView.swift in Profile/Edit
ProfileEditView.swift is correctly placed in the Edit subgroup of Profile.
2079-2079: Group inclusion of UIViewController+KeyboardHandling.swift in Extension
UIViewController+KeyboardHandling.swift is properly included in the Extension group under Helper.
2745-2745: Sources build phase references updated
New source file references for ProfileEditView.swift, TeamCollectionView.swift, and UIViewController+KeyboardHandling.swift are correctly added to the sources build phase.Also applies to: 2884-2884, 2919-2919
| lazy var doneButton: WableButton = WableButton(style: .primary).then { | ||
| $0.clipsToBounds = true | ||
| $0.layer.cornerRadius = 16.adjustedHeight | ||
| $0.configuration?.attributedTitle = "완료".pretendardString(with: .body3) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
doneButton initialisation: guard against missing configuration
WableButton(style:) presumably creates a UIButton.Configuration, but if that implementation ever returns nil (e.g. pre-iOS 15 fallback), the current optional-chaining silently does nothing, leaving the title empty.
- $0.configuration?.attributedTitle = "완료".pretendardString(with: .body3)
+ if var config = $0.configuration {
+ config.attributedTitle = "완료".pretendardString(with: .body3)
+ $0.configuration = config
+ } else {
+ $0.setAttributedTitle(
+ "완료".pretendardString(with: .body3), for: .normal
+ )
+ }
+
+ // Accessibility
+ $0.accessibilityLabel = NSLocalizedString("완료", comment: "완료 버튼")📝 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.
| lazy var doneButton: WableButton = WableButton(style: .primary).then { | |
| $0.clipsToBounds = true | |
| $0.layer.cornerRadius = 16.adjustedHeight | |
| $0.configuration?.attributedTitle = "완료".pretendardString(with: .body3) | |
| } | |
| lazy var doneButton: WableButton = WableButton(style: .primary).then { | |
| $0.clipsToBounds = true | |
| $0.layer.cornerRadius = 16.adjustedHeight | |
| if var config = $0.configuration { | |
| config.attributedTitle = "완료".pretendardString(with: .body3) | |
| $0.configuration = config | |
| } else { | |
| $0.setAttributedTitle( | |
| "완료".pretendardString(with: .body3), for: .normal | |
| ) | |
| } | |
| // Accessibility | |
| $0.accessibilityLabel = NSLocalizedString("완료", comment: "완료 버튼") | |
| } |
🤖 Prompt for AI Agents
In Wable-iOS/Presentation/WableComponent/Navigation/NavigationView.swift around
lines 109 to 113, the doneButton initialization uses optional chaining on
configuration which may be nil, causing the title to be unset silently. To fix
this, explicitly unwrap or guard the configuration property before setting the
attributedTitle, and provide a fallback or create a new configuration if it is
nil, ensuring the button title is always set correctly.
| func enableKeyboardHandling() { | ||
| NotificationCenter.default.addObserver( | ||
| forName: UIResponder.keyboardWillShowNotification, | ||
| object: nil, | ||
| queue: .main | ||
| ) { [weak self] notification in | ||
| self?.handleKeyboardShow(notification: notification) | ||
| } | ||
|
|
||
| NotificationCenter.default.addObserver( | ||
| forName: UIResponder.keyboardWillHideNotification, | ||
| object: nil, | ||
| queue: .main | ||
| ) { [weak self] notification in | ||
| self?.handleKeyboardHide(notification: notification) | ||
| } | ||
| } | ||
|
|
||
| func disableKeyboardHandling() { | ||
| NotificationCenter.default.removeObserver(self) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Observer tokens are leaked – store and remove them explicitly.
addObserver(forName:object:queue:using:) returns an NSObjectProtocol token that must be retained and later passed to removeObserver(_:).
Calling removeObserver(self) does nothing here, so the closure stays registered → retain-cycle with the captured self, memory leak, and handler still firing after VC deallocation.
Store the two tokens in properties like private var kbObservers: [NSObjectProtocol] = [] and unregister them in deinit, or switch to the selector-based addObserver(_:selector:…) API.
🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 33-33: An object should only remove itself as an observer in deinit
(notification_center_detachment)
🤖 Prompt for AI Agents
In
Wable-iOS/Presentation/Helper/Extension/UIViewController+KeyboardHandling.swift
between lines 14 and 34, the keyboard notification observers are added using
closure-based API but their tokens are not stored, causing memory leaks and
retain cycles. Fix this by adding a private property to store the returned
NSObjectProtocol tokens from addObserver calls, then use these tokens to
explicitly remove the observers in disableKeyboardHandling or deinit.
Alternatively, switch to the selector-based addObserver API which does not
require token management.
| nextButton.snp.makeConstraints { | ||
| $0.top.equalTo(skipButton.snp.bottom).offset(12) | ||
| $0.horizontalEdges.equalToSuperview().inset(16) | ||
| $0.bottom.equalToSuperview().inset(64) | ||
| $0.adjustedHeightEqualTo(56) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Respect safe-area when pinning bottom buttons.
$0.bottom.equalToSuperview().inset(64) anchors nextButton to the view’s bottom, which collides with the home-indicator on notched devices.
Pin to view.safeAreaLayoutGuide (or .safeAreaInsets) instead to avoid overlap.
🤖 Prompt for AI Agents
In Wable-iOS/Presentation/Onboarding/ViewController/LCKTeamViewController.swift
around lines 102 to 107, the nextButton's bottom constraint is pinned directly
to the superview's bottom, which can overlap with the home indicator on devices
with notches. Update the bottom constraint to pin nextButton to the view's
safeAreaLayoutGuide bottom anchor instead of the superview bottom to respect the
safe area and avoid UI overlap.
| lazy var teamCollectionView: TeamCollectionView = { | ||
| return TeamCollectionView(cellDidTapped: { [weak self] selectedTeam in | ||
| guard let self = self else { return } | ||
|
|
||
| myTeamLabel.text = selectedTeam | ||
| cellTapped?(selectedTeam) | ||
| }) | ||
| }() |
There was a problem hiding this comment.
No hook to enable “완료” after team change.
The closure updates myTeamLabel but does not notify ProfileEditViewController to re-evaluate and enable the doneButton.
Expose a delegate or call back to update button state, otherwise users cannot save a team-only change (the controller’s updateDoneButtonState is never hit).
🤖 Prompt for AI Agents
In Wable-iOS/Presentation/Profile/Edit/ProfileEditView.swift around lines 68 to
75, the closure updating myTeamLabel does not notify ProfileEditViewController
to re-evaluate and enable the doneButton. Fix this by adding a delegate method
or callback from the closure that informs the controller to call
updateDoneButtonState, ensuring the doneButton is enabled after a team change.
| $0.trailing.equalTo(profileImageView.snp.trailing).offset(19) | ||
| $0.bottom.equalTo(profileImageView.snp.bottom).offset(6) | ||
| $0.size.equalTo(48.adjustedWidth) | ||
| } | ||
|
|
||
| nicknameDiscriptionLabel.snp.makeConstraints { | ||
| $0.top.equalTo(profileImageView.snp.bottom).offset(6) | ||
| $0.leading.equalToSuperview().offset(16) | ||
| } | ||
|
|
||
| nickNameTextField.snp.makeConstraints { | ||
| $0.top.equalTo(nicknameDiscriptionLabel.snp.bottom).offset(4) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
uppercased needs () — potential compile error.
defaultImage = rootView.defaultImageList[0].uppercased is missing parentheses (uppercased()) unless you added a computed property. Swift standard library only provides the method form.
🤖 Prompt for AI Agents
In Wable-iOS/Presentation/Profile/Edit/ProfileEditView.swift around lines 148 to
159, the code uses `uppercased` without parentheses on `defaultImage =
rootView.defaultImageList[0].uppercased`, which will cause a compile error. Fix
this by adding parentheses to call the method properly: change `uppercased` to
`uppercased()`.
|
|
||
| final class TeamCollectionView: UICollectionView { |
There was a problem hiding this comment.
컬렉션뷰를 컴포넌트화 하는 것은 전혀 생각해보지도 못했던 것 같아요.
결국 내용은 고정되고, 다만 컬렉션뷰의 기능을 이용하기 위해서 이렇게 구현하신 것이겠죠?
이러한 방향성도 괜찮은 것 같네요.
There was a problem hiding this comment.
맞습니다! UI와 기능이 모든 화면에서 동일하게 작동하기 때문에 그런 중복을 줄여보고 싶었어요.
그런데 구현하면서 결국 collectionView도 하나의 view인데 컴포넌트 내에서 기능을 구현하는 게 맞나 싶기도 했습니닷.
Delegate도 사실 collectionView의 기능을 다른 곳에서 위임받아 구현할 수 있도록 만들어둔건데 collectionView에서 직접 구현하니 cellTapped() 처럼 클로저를 받아 넣어주는 부분이 무조건 필요해져서... 좋은 코드인가? 라는 생각이 들었던 것 같아요.
View에서 기능을 구현해도 되는지, 컴포넌트화는 어디까지 하는 게 맞는지, 만약 정말 UI적 요소만 컴포넌트화를 해야 한다면 중복되는 기능은 어떻게 합치면 좋을지에 대해 이야기해보면 좋을 것 같습니다!
| func collectionView(_ collectionView: UICollectionView, numberOfItemsInSection section: Int) -> Int { | ||
| return 10 |
There was a problem hiding this comment.
randomTeamList.count가 실제 데이터소스의 배열에 의존해서 의미 있어 보여요.
There was a problem hiding this comment.
헉 그렇네요. 팀 수는 바뀔 일이 없다고 생각해서 상수로 박아놨는데... 수정하겠습니닷.
| .sink { _ in | ||
| } receiveValue: { owner, _ in |
There was a problem hiding this comment.
receiveCompletion에 해당하는 클로저처럼 보이는데, 사용하지 않는다면 생략해도 좋을 것 같네요.
| @objc func doneButtonDidTap() { | ||
| guard let profile = sessionProfile else { return } | ||
|
|
||
| let nicknameText = rootView.nickNameTextField.text ?? "" | ||
| let hasNicknameChanged = !nicknameText.isEmpty && nicknameText != profile.user.nickname | ||
| let hasImageChanged = defaultImage != nil || hasUserSelectedImage | ||
| let hasTeamChanged = lckTeam != (profile.user.fanTeam?.rawValue ?? "LCK") | ||
|
|
||
| if hasNicknameChanged || hasImageChanged || hasTeamChanged { | ||
| let finalNickname = hasNicknameChanged ? nicknameText : profile.user.nickname | ||
|
|
||
| profileUseCase.execute( | ||
| profile: UserProfile( | ||
| user: User( | ||
| id: profile.user.id, | ||
| nickname: finalNickname, | ||
| profileURL: profile.user.profileURL, | ||
| fanTeam: LCKTeam(rawValue: lckTeam) | ||
| ), | ||
| introduction: profile.introduction, | ||
| ghostCount: profile.ghostCount, | ||
| lckYears: profile.lckYears, | ||
| userLevel: profile.userLevel | ||
| ), | ||
| image: defaultImage == nil ? rootView.profileImageView.image : nil, | ||
| defaultProfileType: defaultImage | ||
| ) |
There was a problem hiding this comment.
한 번 눌리고 다음 스텝으로 넘어가겠지만, 그럼에도 불구하고 doneButton이 눌릴 때마다 새로운 관계가 저장될 것이기 때문에, 기존의 관계는 누수로 이어질 수도 있을 것 같아서 고려해 보시면 좋을 것 같아요.
| var defaultImageList = [ | ||
| DefaultProfileType.blue, | ||
| DefaultProfileType.green, | ||
| DefaultProfileType.purple | ||
| ] |
There was a problem hiding this comment.
DefaultProfileType이 CaseIterable을 채택하여, allCases 프로퍼티를 활용해도 좋을 것 같아요.
There was a problem hiding this comment.
오 이런 방법이... 적용했습니다 감사합니다!
| .sink(receiveCompletion: { [weak self] completion in | ||
| let condition = completion == .finished | ||
|
|
||
| self?.rootView.conditiionLabel.text = condition ? StringLiterals.ProfileSetting.checkVaildMessage : StringLiterals.ProfileSetting.checkDuplicateError | ||
| self?.rootView.conditiionLabel.textColor = condition ? .success : .error | ||
| self?.rootView.nextButton.isUserInteractionEnabled = condition | ||
| self?.rootView.nextButton.updateStyle(condition ? .primary : .gray) | ||
| self?.rootView.conditionLabel.text = condition ? StringLiterals.ProfileSetting.checkVaildMessage : StringLiterals.ProfileSetting.checkDuplicateError | ||
| self?.rootView.conditionLabel.textColor = condition ? .success : .error | ||
| self?.navigationView.doneButton.isUserInteractionEnabled = condition | ||
| self?.navigationView.doneButton.updateStyle(condition ? .primary : .gray) | ||
| }, receiveValue: { _ in | ||
| }) |
There was a problem hiding this comment.
일반적이라면, receiveValue에서 동작할 것 같은데요.
receiveCompletion에서 위와 같은 작업을 수행하도록 선언하신 이유가 궁금합니다.
There was a problem hiding this comment.
성공 여부에 따라 같은 컴포넌트에 서로 다른 UI 처리를 적용해주어야 했고, 이를 처리하기 위해서는 성공 여부를 확인 가능한 receiveCompletion에서 처리해주는 것이 더 나을 것이라고 판단했습니다!
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
Wable-iOS/Presentation/Profile/Edit/ProfileEditViewController.swift (1)
271-282: Update done button state should consider team changes.The
updateDoneButtonState()method only checks for nickname and image changes but ignores team changes, which could leave the button disabled when only the team is modified.func updateDoneButtonState() { guard let profile = sessionProfile else { return } let nicknameText = rootView.nickNameTextField.text ?? "" let hasNicknameChanged = !nicknameText.isEmpty && nicknameText != profile.user.nickname let hasImageChanged = defaultImage != nil || hasUserSelectedImage + let hasTeamChanged = lckTeam != (profile.user.fanTeam?.rawValue ?? "LCK") - let shouldEnable = hasNicknameChanged || hasImageChanged + let shouldEnable = hasNicknameChanged || hasImageChanged || hasTeamChanged navigationView.doneButton.isUserInteractionEnabled = shouldEnable navigationView.doneButton.updateStyle(shouldEnable ? .primary : .gray) }
♻️ Duplicate comments (1)
Wable-iOS/Presentation/Profile/Edit/ProfileEditView.swift (1)
64-71: Team selection callback doesn't enable done button.The closure updates
myTeamLabelbut doesn't notify the controller to re-evaluate the done button state. Users won't be able to save team-only changes sinceupdateDoneButtonState()is never called.
🧹 Nitpick comments (4)
Wable-iOS/Presentation/Profile/Edit/ProfileEditView.swift (1)
102-106: Consider thread safety for UI updates.The
configureDefaultImage()method modifies thedefaultImageListarray and updates UI elements. If called from a background thread, this could cause issues.Consider ensuring main thread execution:
func configureDefaultImage() { + DispatchQueue.main.async { [weak self] in + guard let self = self else { return } defaultImageList.shuffle() profileImageView.image = UIImage(named: defaultImageList[0].rawValue) + } }Wable-iOS/Presentation/WableComponent/View/TeamCollectionView.swift (2)
72-77: Optimize enumeration when index isn't needed.The static analysis tool correctly identifies that the enumerated index isn't used in this loop.
-for (index, _) in randomTeamList.enumerated() { - if let cell = cellForItem(at: IndexPath(row: index, section: 0)) as? LCKTeamCollectionViewCell { +for index in randomTeamList.indices { + if let cell = cellForItem(at: IndexPath(row: index, section: 0)) as? LCKTeamCollectionViewCell { cell.layer.borderColor = UIColor.gray300.cgColor cell.teamLabel.textColor = .gray700 } }
58-86: Consider removing unnecessary async dispatch.The
selectInitialTeammethod usesDispatchQueue.main.asyncbut may already be called from the main thread. This could be optimized or at least documented why async dispatch is necessary.func selectInitialTeam(team: LCKTeam?) { guard let team = team else { return } selectedTeam = team - DispatchQueue.main.async { [weak self] in - guard let self = self, - let selectedTeam = selectedTeam, - let index = randomTeamList.firstIndex(of: selectedTeam) else { - return - } + guard let index = randomTeamList.firstIndex(of: team) else { return } + + let indexPath = IndexPath(row: index, section: 0) + + // Reset all cells to default state + for index in randomTeamList.indices { + if let cell = cellForItem(at: IndexPath(row: index, section: 0)) as? LCKTeamCollectionViewCell { + cell.layer.borderColor = UIColor.gray300.cgColor + cell.teamLabel.textColor = .gray700 + } + } + + // Highlight selected cell + if let cell = cellForItem(at: indexPath) as? LCKTeamCollectionViewCell { + cell.layer.borderColor = UIColor.purple50.cgColor + cell.teamLabel.textColor = .wableBlack + } - let indexPath = IndexPath(row: index, section: 0) - - for index in randomTeamList.indices { - if let cell = cellForItem(at: IndexPath(row: index, section: 0)) as? LCKTeamCollectionViewCell { - cell.layer.borderColor = UIColor.gray300.cgColor - cell.teamLabel.textColor = .gray700 - } - } - - if let cell = cellForItem(at: indexPath) as? LCKTeamCollectionViewCell { - cell.layer.borderColor = UIColor.purple50.cgColor - cell.teamLabel.textColor = .wableBlack - } - - selectItem(at: indexPath, animated: false, scrollPosition: []) - } + selectItem(at: indexPath, animated: false, scrollPosition: []) }Wable-iOS/Presentation/Profile/Edit/ProfileEditViewController.swift (1)
30-31: Remove redundant nil initialization.Swift optionals are automatically initialized to
nil, making explicit initialization redundant.-private var sessionProfile: UserProfile? = nil -private var defaultImage: String? = nil +private var sessionProfile: UserProfile? +private var defaultImage: String?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
Wable-iOS/Presentation/Enum/DefaultProfileType.swift(1 hunks)Wable-iOS/Presentation/Profile/Edit/ProfileEditView.swift(1 hunks)Wable-iOS/Presentation/Profile/Edit/ProfileEditViewController.swift(9 hunks)Wable-iOS/Presentation/WableComponent/View/TeamCollectionView.swift(1 hunks)
🧰 Additional context used
🪛 SwiftLint (0.57.0)
Wable-iOS/Presentation/Profile/Edit/ProfileEditViewController.swift
[Warning] 30-30: Initializing an optional variable with nil is redundant
(redundant_optional_initialization)
[Warning] 31-31: Initializing an optional variable with nil is redundant
(redundant_optional_initialization)
Wable-iOS/Presentation/WableComponent/View/TeamCollectionView.swift
[Warning] 72-72: When the item is not used, .indices should be used instead of .enumerated()
(unused_enumerated)
🔇 Additional comments (4)
Wable-iOS/Presentation/Enum/DefaultProfileType.swift (1)
11-11: LGTM! Clean enum enhancement.Adding
CaseIterableconformance enables the use ofallCasesproperty, which improves the enum's usability and aligns well with the new profile editing components that need to iterate over all profile types.Wable-iOS/Presentation/Profile/Edit/ProfileEditView.swift (1)
17-17: Excellent use of the new CaseIterable conformance.Using
DefaultProfileType.allCasesleverages the enum enhancement from the first file, making the code more maintainable and eliminating the need to manually list all cases.Wable-iOS/Presentation/WableComponent/View/TeamCollectionView.swift (2)
10-36: Solid componentization approach for reusable team selection.The encapsulation of collection view logic with closure-based callbacks is a good design choice that eliminates code duplication across screens.
113-115: Good use of dynamic count instead of hardcoded value.Using
randomTeamList.countmakes the code more maintainable than a hardcoded constant, as it automatically adapts if the team list changes.
| func configureView(profileImageURL: URL? = .none, team: LCKTeam?) { | ||
| myTeamLabel.text = team?.rawValue ?? "LCK" | ||
| teamCollectionView.selectInitialTeam(team: team) | ||
|
|
||
| if let profileImageURL = profileImageURL { | ||
| switch profileImageURL.absoluteString { | ||
| case "PURPLE": | ||
| profileImageView.image = .imgProfilePurple | ||
| case "GREEN": | ||
| profileImageView.image = .imgProfileGreen | ||
| case "BLUE": | ||
| profileImageView.image = .imgProfileBlue | ||
| default: | ||
| profileImageView.kf.setImage( | ||
| with: profileImageURL, | ||
| placeholder: [UIImage.imgProfilePurple, UIImage.imgProfileBlue, UIImage.imgProfileGreen].randomElement() | ||
| ) | ||
| } | ||
| } else { | ||
| configureDefaultImage() | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve the profile image URL handling logic.
The hardcoded string comparisons for profile URLs ("PURPLE", "GREEN", "BLUE") are fragile and could break if the data format changes. Consider using the DefaultProfileType enum for type safety.
func configureView(profileImageURL: URL? = .none, team: LCKTeam?) {
myTeamLabel.text = team?.rawValue ?? "LCK"
teamCollectionView.selectInitialTeam(team: team)
if let profileImageURL = profileImageURL {
- switch profileImageURL.absoluteString {
- case "PURPLE":
- profileImageView.image = .imgProfilePurple
- case "GREEN":
- profileImageView.image = .imgProfileGreen
- case "BLUE":
- profileImageView.image = .imgProfileBlue
- default:
+ if let profileType = DefaultProfileType.allCases.first(where: { $0.uppercased == profileImageURL.absoluteString }) {
+ profileImageView.image = UIImage(named: profileType.rawValue)
+ } else {
profileImageView.kf.setImage(
with: profileImageURL,
placeholder: [UIImage.imgProfilePurple, UIImage.imgProfileBlue, UIImage.imgProfileGreen].randomElement()
)
- }
+ }
} else {
configureDefaultImage()
}
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In Wable-iOS/Presentation/Profile/Edit/ProfileEditView.swift around lines 195 to
217, replace the hardcoded string comparisons for profileImageURL with a safer
approach using the DefaultProfileType enum. Parse or map the profileImageURL to
a DefaultProfileType value and switch on that enum instead of raw strings. This
will improve type safety and maintainability by avoiding fragile string
literals.
| navigationView.doneButton | ||
| .publisher(for: .touchUpInside) | ||
| .sink { [weak self] _ in | ||
| guard let self = self, | ||
| let profile = sessionProfile | ||
| else { | ||
| return | ||
| } | ||
|
|
||
| let nicknameText = rootView.nickNameTextField.text ?? "" | ||
| let hasNicknameChanged = !nicknameText.isEmpty && nicknameText != profile.user.nickname | ||
| let hasImageChanged = defaultImage != nil || hasUserSelectedImage | ||
| let hasTeamChanged = lckTeam != (profile.user.fanTeam?.rawValue ?? "LCK") | ||
|
|
||
| if hasNicknameChanged || hasImageChanged || hasTeamChanged { | ||
| let finalNickname = hasNicknameChanged ? nicknameText : profile.user.nickname | ||
|
|
||
| profileUseCase.execute( | ||
| profile: UserProfile( | ||
| user: User( | ||
| id: profile.user.id, | ||
| nickname: finalNickname, | ||
| profileURL: profile.user.profileURL, | ||
| fanTeam: LCKTeam(rawValue: lckTeam) | ||
| ), | ||
| introduction: profile.introduction, | ||
| ghostCount: profile.ghostCount, | ||
| lckYears: profile.lckYears, | ||
| userLevel: profile.userLevel | ||
| ), | ||
| image: defaultImage == nil ? rootView.profileImageView.image : nil, | ||
| defaultProfileType: defaultImage | ||
| ) | ||
| .replaceError(with: ()) | ||
| .withUnretained(self) | ||
| .sink(receiveValue: { owner, _ in | ||
| owner.navigationController?.popViewController(animated: true) | ||
| }) | ||
| .store(in: cancelBag) | ||
| } else { | ||
| navigationController?.popViewController(animated: true) | ||
| } | ||
| } | ||
| .store(in: cancelBag) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Complex inline closure should be extracted to a method.
The done button setup contains complex logic that would be better extracted to a separate method for readability and maintainability.
func setupAction() {
rootView.switchButton.addTarget(self, action: #selector(switchButtonDidTap), for: .touchUpInside)
rootView.addButton.addTarget(self, action: #selector(addButtonDidTap), for: .touchUpInside)
rootView.duplicationCheckButton.addTarget(self, action: #selector(duplicationCheckButtonDidTap), for: .touchUpInside)
- navigationView.doneButton
- .publisher(for: .touchUpInside)
- .sink { [weak self] _ in
- guard let self = self,
- let profile = sessionProfile
- else {
- return
- }
-
- let nicknameText = rootView.nickNameTextField.text ?? ""
- let hasNicknameChanged = !nicknameText.isEmpty && nicknameText != profile.user.nickname
- let hasImageChanged = defaultImage != nil || hasUserSelectedImage
- let hasTeamChanged = lckTeam != (profile.user.fanTeam?.rawValue ?? "LCK")
-
- if hasNicknameChanged || hasImageChanged || hasTeamChanged {
- let finalNickname = hasNicknameChanged ? nicknameText : profile.user.nickname
-
- profileUseCase.execute(
- profile: UserProfile(
- user: User(
- id: profile.user.id,
- nickname: finalNickname,
- profileURL: profile.user.profileURL,
- fanTeam: LCKTeam(rawValue: lckTeam)
- ),
- introduction: profile.introduction,
- ghostCount: profile.ghostCount,
- lckYears: profile.lckYears,
- userLevel: profile.userLevel
- ),
- image: defaultImage == nil ? rootView.profileImageView.image : nil,
- defaultProfileType: defaultImage
- )
- .replaceError(with: ())
- .withUnretained(self)
- .sink(receiveValue: { owner, _ in
- owner.navigationController?.popViewController(animated: true)
- })
- .store(in: cancelBag)
- } else {
- navigationController?.popViewController(animated: true)
- }
- }
- .store(in: cancelBag)
+ navigationView.doneButton
+ .publisher(for: .touchUpInside)
+ .sink { [weak self] _ in
+ self?.handleDoneButtonTap()
+ }
+ .store(in: cancelBag)
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In Wable-iOS/Presentation/Profile/Edit/ProfileEditViewController.swift around
lines 102 to 145, the closure handling the doneButton tap contains complex logic
that reduces readability. Extract this closure's entire logic into a separate
private method within the view controller, then call that method from the sink
closure. This will improve code clarity and maintainability by isolating the
button action handling.
| @objc func doneButtonDidTap() { | ||
| guard let profile = sessionProfile else { return } | ||
|
|
||
| let nicknameText = rootView.nickNameTextField.text ?? "" | ||
| let hasNicknameChanged = !nicknameText.isEmpty && nicknameText != profile.user.nickname | ||
| let hasImageChanged = defaultImage != nil || hasUserSelectedImage | ||
| let hasTeamChanged = lckTeam != (profile.user.fanTeam?.rawValue ?? "LCK") | ||
|
|
||
| if hasNicknameChanged || hasImageChanged || hasTeamChanged { | ||
| let finalNickname = hasNicknameChanged ? nicknameText : profile.user.nickname | ||
|
|
||
| profileUseCase.execute( | ||
| profile: UserProfile( | ||
| user: User( | ||
| id: profile.user.id, | ||
| nickname: finalNickname, | ||
| profileURL: profile.user.profileURL, | ||
| fanTeam: LCKTeam(rawValue: lckTeam) | ||
| ), | ||
| introduction: profile.introduction, | ||
| ghostCount: profile.ghostCount, | ||
| lckYears: profile.lckYears, | ||
| userLevel: profile.userLevel | ||
| ), | ||
| image: defaultImage == nil ? rootView.profileImageView.image : nil, | ||
| defaultProfileType: defaultImage | ||
| ) | ||
| .replaceError(with: ()) | ||
| .withUnretained(self) | ||
| .sink(receiveValue: { owner, _ in | ||
| owner.navigationController?.popViewController(animated: true) | ||
| }) | ||
| .store(in: cancelBag) | ||
| } else { | ||
| navigationController?.popViewController(animated: true) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove duplicate method implementation.
The doneButtonDidTap method appears to be dead code as it's not referenced anywhere, and its logic is duplicated in the setupAction method's closure.
-@objc func doneButtonDidTap() {
- guard let profile = sessionProfile else { return }
-
- let nicknameText = rootView.nickNameTextField.text ?? ""
- let hasNicknameChanged = !nicknameText.isEmpty && nicknameText != profile.user.nickname
- let hasImageChanged = defaultImage != nil || hasUserSelectedImage
- let hasTeamChanged = lckTeam != (profile.user.fanTeam?.rawValue ?? "LCK")
-
- if hasNicknameChanged || hasImageChanged || hasTeamChanged {
- let finalNickname = hasNicknameChanged ? nicknameText : profile.user.nickname
-
- profileUseCase.execute(
- profile: UserProfile(
- user: User(
- id: profile.user.id,
- nickname: finalNickname,
- profileURL: profile.user.profileURL,
- fanTeam: LCKTeam(rawValue: lckTeam)
- ),
- introduction: profile.introduction,
- ghostCount: profile.ghostCount,
- lckYears: profile.lckYears,
- userLevel: profile.userLevel
- ),
- image: defaultImage == nil ? rootView.profileImageView.image : nil,
- defaultProfileType: defaultImage
- )
- .replaceError(with: ())
- .withUnretained(self)
- .sink(receiveValue: { owner, _ in
- owner.navigationController?.popViewController(animated: true)
- })
- .store(in: cancelBag)
- } else {
- navigationController?.popViewController(animated: true)
- }
-}If this method is intended to be the extracted implementation from the inline closure, rename it to handleDoneButtonTap() and remove the @objc since it's not used as a selector target.
📝 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.
| @objc func doneButtonDidTap() { | |
| guard let profile = sessionProfile else { return } | |
| let nicknameText = rootView.nickNameTextField.text ?? "" | |
| let hasNicknameChanged = !nicknameText.isEmpty && nicknameText != profile.user.nickname | |
| let hasImageChanged = defaultImage != nil || hasUserSelectedImage | |
| let hasTeamChanged = lckTeam != (profile.user.fanTeam?.rawValue ?? "LCK") | |
| if hasNicknameChanged || hasImageChanged || hasTeamChanged { | |
| let finalNickname = hasNicknameChanged ? nicknameText : profile.user.nickname | |
| profileUseCase.execute( | |
| profile: UserProfile( | |
| user: User( | |
| id: profile.user.id, | |
| nickname: finalNickname, | |
| profileURL: profile.user.profileURL, | |
| fanTeam: LCKTeam(rawValue: lckTeam) | |
| ), | |
| introduction: profile.introduction, | |
| ghostCount: profile.ghostCount, | |
| lckYears: profile.lckYears, | |
| userLevel: profile.userLevel | |
| ), | |
| image: defaultImage == nil ? rootView.profileImageView.image : nil, | |
| defaultProfileType: defaultImage | |
| ) | |
| .replaceError(with: ()) | |
| .withUnretained(self) | |
| .sink(receiveValue: { owner, _ in | |
| owner.navigationController?.popViewController(animated: true) | |
| }) | |
| .store(in: cancelBag) | |
| } else { | |
| navigationController?.popViewController(animated: true) | |
| } | |
| } |
🤖 Prompt for AI Agents
In Wable-iOS/Presentation/Profile/Edit/ProfileEditViewController.swift between
lines 161 and 197, the doneButtonDidTap method duplicates logic already present
in the setupAction method's closure and is not referenced elsewhere. Remove this
duplicate method entirely if unused, or if it is meant to replace the closure
logic, rename it to handleDoneButtonTap and remove the @objc attribute since it
is not used as a selector target.
| private lazy var rootView = ProfileEditView(cellTapped: { [weak self] selectedTeam in | ||
| guard let self = self else { return } | ||
|
|
||
| lckTeam = selectedTeam | ||
| }) |
There was a problem hiding this comment.
Team selection callback missing done button update.
The closure updates lckTeam but doesn't call updateDoneButtonState() to enable the done button when only the team changes. This prevents users from saving team-only modifications.
private lazy var rootView = ProfileEditView(cellTapped: { [weak self] selectedTeam in
guard let self = self else { return }
lckTeam = selectedTeam
+ updateDoneButtonState()
})📝 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.
| private lazy var rootView = ProfileEditView(cellTapped: { [weak self] selectedTeam in | |
| guard let self = self else { return } | |
| lckTeam = selectedTeam | |
| }) | |
| private lazy var rootView = ProfileEditView(cellTapped: { [weak self] selectedTeam in | |
| guard let self = self else { return } | |
| lckTeam = selectedTeam | |
| updateDoneButtonState() | |
| }) |
🤖 Prompt for AI Agents
In Wable-iOS/Presentation/Profile/Edit/ProfileEditViewController.swift around
lines 36 to 40, the closure for the team selection callback updates the lckTeam
variable but does not call updateDoneButtonState(), which is needed to enable
the done button when only the team changes. Modify the closure to call
updateDoneButtonState() after setting lckTeam to ensure the done button state
reflects the team selection change.
[Feat] 프로필 수정 기능 및 키보드 표시/숨김 처리에 따른 뷰 핸들링 익스텐션 구현
👻 PULL REQUEST
📄 작업 내용
CollectionView를TeamCollectionView로 컴포넌트화해 구현했습니다.UIViewController의 익스텐션으로 키보드 핸들링 기능을 구현했습니다.💻 주요 코드 설명
키보드 표시/숨김 처리에 따른 뷰 핸들링 익스텐션 구현
UIScrollView와 컨테이너로 사용할UIView가 추가적으로 필요해 기존 화면의 수정이 필요하고, 핸들링이 필요한 모든 뷰에 추가가 필요하다는 점 때문에 코드 변경이 적은 제약조건 수정 방식으로 구현해본 것도 있습니다.👀 기타 더 이야기해볼 점
🔗 연결된 이슈
Summary by CodeRabbit
New Features
Improvements
Bug Fixes