-
Notifications
You must be signed in to change notification settings - Fork 0
[Refactor] String 익스텐션 수정 #157
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
Conversation
WalkthroughThe pull request updates the Xcode project file to reintroduce a previously removed file reference and add a new one. It refactors attributed string utility methods by replacing the old underline method with a new one and adding text highlighting capabilities in both Changes
Assessment against linked issues
Suggested labels
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)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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: 2
🧹 Nitpick comments (2)
Wable-iOS/Presentation/Helper/Extension/AttributedString+.swift (2)
65-79: Add error handling for when target text is not foundThe method handles empty strings correctly but might benefit from improved error handling when the target text isn't found in the string.
Consider adding a comment or return value that indicates when the target text wasn't found:
func addUnderline(to targetText: String, style: NSUnderlineStyle = .single) -> AttributedString { guard !self.characters.isEmpty, !targetText.isEmpty else { return self } var newString = self if let range = newString.range(of: targetText) { var container = AttributeContainer() container.underlineStyle = style newString[range].mergeAttributes(container) + } else { + // Target text not found, returning original string } return newString }
101-114: Add error handling for when target text is not foundSimilar to the
addUnderline(to:)method, consider improving error handling when the target text isn't found in the string.Consider adding a comment or return value that indicates when the target text wasn't found:
func highlight(textColor: UIColor, to targetText: String) -> AttributedString { guard !self.characters.isEmpty, !targetText.isEmpty else { return self } var newString = self if let range = newString.range(of: targetText) { var container = AttributeContainer() container.foregroundColor = textColor newString[range].mergeAttributes(container) + } else { + // Target text not found, returning original string } return newString }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
Wable-iOS.xcodeproj/project.pbxproj(4 hunks)Wable-iOS/Presentation/Helper/Extension/AttributedString+.swift(1 hunks)Wable-iOS/Presentation/Helper/Extension/NSMutableAttributedString+.swift(1 hunks)Wable-iOS/Presentation/Helper/Extension/String+.swift(3 hunks)Wable-iOS/Presentation/Onboarding/View/AgreementItemView.swift(1 hunks)Wable-iOS/Presentation/Overview/Rank/View/RankListViewController.swift(1 hunks)
🧰 Additional context used
🧬 Code Definitions (2)
Wable-iOS/Presentation/Helper/Extension/AttributedString+.swift (1)
Wable-iOS/Presentation/Helper/Extension/NSMutableAttributedString+.swift (3)
addUnderline(31-39)addUnderline(60-71)highlight(94-105)
Wable-iOS/Presentation/Helper/Extension/NSMutableAttributedString+.swift (1)
Wable-iOS/Presentation/Helper/Extension/AttributedString+.swift (3)
addUnderline(31-43)addUnderline(64-79)highlight(100-115)
🔇 Additional comments (14)
Wable-iOS/Presentation/Onboarding/View/AgreementItemView.swift (1)
26-26: Good API update to improve text styling consistency.The change from
withUnderline()toaddUnderline()aligns with the PR objective of refactoring string extensions. The new method name is more action-oriented and follows a consistent naming pattern with other text styling methods in the refactored implementation.Wable-iOS/Presentation/Overview/Rank/View/RankListViewController.swift (1)
46-48: Excellent refactoring for improved readability and maintainability.The implementation now uses the new extension methods to create attributed text with highlighting in a much cleaner way. This change:
- Simplifies the code by removing manual attribute range manipulation
- Improves readability with a declarative API
- Makes the styling more consistent with the rest of the application
The new
highlight()method provides a clear, concise way to apply text styling that aligns perfectly with the PR objectives.Wable-iOS.xcodeproj/project.pbxproj (6)
250-250: File reintroduced to support UI component updates.The
Publisher+Driver.swiftfile has been reintroduced to the project, which is necessary to support the string extension refactoring mentioned in the PR objectives.
255-255: New extension file added for attributed string enhancements.The
NSMutableAttributedString+.swiftfile has been added to the project, which aligns with the PR objective of adding new extension methods forNSMutableAttributedString.
503-503: File reference added properly.The file reference for
Publisher+Driver.swifthas been correctly added to the project, ensuring it's properly tracked in the project structure.
508-508: New extension file reference added correctly.The file reference for
NSMutableAttributedString+.swifthas been properly added to the project structure, which is needed for the new string extension functionality.
1608-1609: Extensions organized in a logical order.The new
NSMutableAttributedString+.swiftextension has been placed alongside the existingAttributedString+.swiftin the Extension group, which maintains a consistent organization of related extension files.
2072-2072: New extension file included in build phases.The
NSMutableAttributedString+.swiftfile has been correctly included in the compilation sources, ensuring it will be built with the project.Wable-iOS/Presentation/Helper/Extension/String+.swift (4)
14-18: Documentation updated correctly to reflect return type changeThe documentation comments have been correctly updated to reflect that the method now returns a
NSMutableAttributedStringinstead of anNSAttributedString.
24-24: Function signature appropriately modifiedThe function signature has been updated to return
NSMutableAttributedString, which aligns with the PR objectives of modifying the return type.
36-36: Return statement correctly updatedThe implementation now directly constructs and returns a
NSMutableAttributedStringinstead of anNSAttributedString, which is consistent with the updated function signature.
52-52: Code simplificationThe implementation has been improved by directly constructing the
AttributedStringfrom the result of the updatedpretendardStringmethod, eliminating unnecessary intermediate variables.Wable-iOS/Presentation/Helper/Extension/AttributedString+.swift (1)
11-43: Well-implemented underline method for the entire textThe
addUnderlinemethod is well-implemented with proper documentation, error handling for empty strings, and an appropriate use of the@discardableResultattribute. The implementation correctly handlesAttributedStringas a value type by creating a new string rather than modifying in place.Wable-iOS/Presentation/Helper/Extension/NSMutableAttributedString+.swift (1)
8-39: Well-documented underline method with proper implementationThe
addUnderlinemethod forNSMutableAttributedStringis well-documented with clear usage examples and proper error handling for empty strings. The@discardableResultattribute is used appropriately to support method chaining.
Wable-iOS/Presentation/Helper/Extension/NSMutableAttributedString+.swift
Show resolved
Hide resolved
Wable-iOS/Presentation/Helper/Extension/NSMutableAttributedString+.swift
Show resolved
Hide resolved
youz2me
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.
고생하셨습니닷 생일까지 열일을 ㅜㅜ
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.
문서 작성 굿입니닷!!
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.
감사합니다!
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.
헉 NSMutableAttributedString랑 AttributedString 둘다 되어있군요
|
|
||
| lazy var infoButton: UIButton = UIButton(configuration: .plain()).then { | ||
| $0.configuration?.attributedTitle = "보러가기".pretendardString(with: .body4).withUnderline() | ||
| $0.configuration?.attributedTitle = "보러가기".pretendardString(with: .body4).addUnderline() |
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.
ㅎㅎ 감사합니다 ~~!!!!
👻 PULL REQUEST
📄 작업 내용
String의 익스텐션인pretendardString의 리턴 타입을NSMutableAttributedString으로 변경하였습니다.NSMutableAttributedString의 익스텐션 메서드를 구현하였습니다.AttributedString의 익스텐션 메서드를 구현하였습니다.💻 주요 코드 설명
NSMutableAttributedString의 익스텐션 메서드
AttributedString도 마찬가지입니다.NSMutableAttributedString+.swift
🔗 연결된 이슈
Summary by CodeRabbit
Chores
New Features
Refactor
Bug Fixes
Removed Features
NavigationViewControllerclass to streamline navigation handling.