-
Couldn't load subscription status.
- Fork 0
Add showOrHideBookmark method to update bookmark button visibility #241
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
base: master
Are you sure you want to change the base?
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.
|
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: 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.
Summary by CodeRabbit