Add showOrHideBookmark method to update bookmark button visibility#241
Add showOrHideBookmark method to update bookmark button visibility#241PruthiviRaj27 wants to merge 1 commit intomasterfrom
Conversation
WalkthroughA new method, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ProfileViewController
participant InstituteRepository
User->>ProfileViewController: viewDidLoad()
ProfileViewController->>ProfileViewController: showOrHideBookmark()
ProfileViewController->>InstituteRepository: getSettings()
InstituteRepository-->>ProfileViewController: settings (bookmarksEnabled)
ProfileViewController->>ProfileViewController: Update bookmarkButtonLayout visibility
Poem
✨ Finishing Touches
🪧 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 review |
✅ Actions performedReview triggered.
|
coderabbitai
bot
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
ios-app/UI/ProfileViewController.swift (1)
87-94: Implementation follows existing patterns well, but consider error handling.The method correctly follows the established pattern used in
showDeleteAccountButtonIfSignupAllowed()with proper weak self capture and main thread UI updates. However, the error parameter from the completion handler is ignored, which could lead to the UI not being updated if the settings fetch fails.Consider adding basic error handling to ensure UI consistency:
func showOrHideBookmark() { InstituteRepository.shared.getSettings { [weak self] settings, error in - guard let settings = settings else { return } + guard let settings = settings else { + // Fallback to default behavior if settings fetch fails + DispatchQueue.main.async { + self?.bookmarkButtonLayout.isHidden = !Constants.BOOKMARKS_ENABLED + } + return + } DispatchQueue.main.async { self?.bookmarkButtonLayout.isHidden = !settings.bookmarksEnabled } } }Optional: Consider extracting common settings fetching pattern.
Both
showOrHideBookmark()andshowDeleteAccountButtonIfSignupAllowed()follow the same pattern. You could create a helper method to reduce duplication:private func updateUIWithSettings(_ updateBlock: @escaping (InstituteSettings) -> Void) { InstituteRepository.shared.getSettings { [weak self] settings, error in guard let settings = settings else { return } DispatchQueue.main.async { updateBlock(settings) } } }Then use it like:
func showOrHideBookmark() { updateUIWithSettings { [weak self] settings in self?.bookmarkButtonLayout.isHidden = !settings.bookmarksEnabled } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ios-app/UI/ProfileViewController.swift(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
ios-app/UI/ProfileViewController.swift (1)
CourseKit/Source/Repostories/InstituteRepository.swift (1)
getSettings(22-32)
🔇 Additional comments (1)
ios-app/UI/ProfileViewController.swift (1)
65-65: Good placement of the method call.The method call is appropriately placed in
viewDidLoadto ensure the bookmark button visibility is updated based on current settings during the view initialization process.
1 participant
Summary by CodeRabbit