Conversation
fixed take 1
Deleted comments to clean up code!
Shrunk fixed button width and shrunk Calendar icon frame to show the full "Closed Today," and shrunk the right button inset to make Change Date button insets more even
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughTwo small UI layout changes: MenuHeaderView's date-change button margins and internal constraints were adjusted (image sized and title trailing relaxed), and HomeViewController's collection view bottom content inset was increased by 25 points beyond the tab bar height. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
Eatery Blue/UI/EateryScreen/MenuHeaderView.swift (1)
102-106: Add spacing between titleLabel and buttonView to prevent them from touching.The
lessThanOrEqualToconstraint correctly prevents overlap, but there's no gap between the title and button. On screens where the title nearly fills the available width, the two elements could visually touch.♻️ Suggested fix
titleLabel.snp.makeConstraints { make in make.top.leading.equalTo(layoutMarginsGuide) - make.trailing.lessThanOrEqualTo(buttonView.snp.leading) + make.trailing.lessThanOrEqualTo(buttonView.snp.leading).offset(-8) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Eatery` Blue/UI/EateryScreen/MenuHeaderView.swift around lines 102 - 106, In setUpConstraints the titleLabel’s trailing uses make.trailing.lessThanOrEqualTo(buttonView.snp.leading) which prevents overlap but allows the views to touch; update the constraint on titleLabel (in setUpConstraints) to add a horizontal gap (e.g., apply a negative offset/inset or constant such as -8 or -12) from buttonView.snp.leading so there is consistent spacing between titleLabel and buttonView and the label truncates before touching the button.Eatery Blue/UI/HomeScreen/HomeViewController.swift (1)
124-124: Consider extracting the magic number to a named constant.The
+ 25offset works to align with theCompareMenusButtoninset, but the inline comment// 108is ambiguous—it's unclear whether it represents the target total or a historical value. For clarity and maintainability, consider adding a named constant inConstants:♻️ Suggested refactor
enum Constants { static let carouselViewLayoutMargins = UIEdgeInsets(top: 6, left: 0, bottom: 6, right: 0) static let customViewLayoutMargins = UIEdgeInsets(top: 6, left: 0, bottom: 6, right: 0) static let minHeaderHeight: CGFloat = 44 static let maxHeaderHeight: CGFloat = 98 static let loadingHeaderHeight: CGFloat = maxHeaderHeight + 52 static let collectionViewTopPadding: CGFloat = 8 static let collectionViewSectionPadding: CGFloat = 16 + static let collectionViewBottomExtraPadding: CGFloat = 25 // Extra padding to clear CompareMenusButton static let isTesting = false }Then at line 124:
- collectionView.contentInset.bottom = (tabBarController?.tabBar.frame.height ?? 0) + 25 // 108 + collectionView.contentInset.bottom = (tabBarController?.tabBar.frame.height ?? 0) + Constants.collectionViewBottomExtraPadding🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Eatery` Blue/UI/HomeScreen/HomeViewController.swift at line 124, Replace the magic "+ 25" in the collection view bottom inset with a named constant to clarify intent: introduce a constant (e.g., CompareMenusButton.bottomInset or HomeViewConstants.compareMenusButtonInset) in your Constants file and use it in the expression that sets collectionView.contentInset.bottom (the line using tabBarController?.tabBar.frame.height). Remove or update the ambiguous "// 108" comment so the constant name documents the purpose (aligning with CompareMenusButton) and update any references to keep behavior identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Eatery` Blue/UI/EateryScreen/MenuHeaderView.swift:
- Around line 102-106: In setUpConstraints the titleLabel’s trailing uses
make.trailing.lessThanOrEqualTo(buttonView.snp.leading) which prevents overlap
but allows the views to touch; update the constraint on titleLabel (in
setUpConstraints) to add a horizontal gap (e.g., apply a negative offset/inset
or constant such as -8 or -12) from buttonView.snp.leading so there is
consistent spacing between titleLabel and buttonView and the label truncates
before touching the button.
In `@Eatery` Blue/UI/HomeScreen/HomeViewController.swift:
- Line 124: Replace the magic "+ 25" in the collection view bottom inset with a
named constant to clarify intent: introduce a constant (e.g.,
CompareMenusButton.bottomInset or HomeViewConstants.compareMenusButtonInset) in
your Constants file and use it in the expression that sets
collectionView.contentInset.bottom (the line using
tabBarController?.tabBar.frame.height). Remove or update the ambiguous "// 108"
comment so the constant name documents the purpose (aligning with
CompareMenusButton) and update any references to keep behavior identical.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9e95eb74-3e68-4353-b8af-f4104f765b3d
📒 Files selected for processing (2)
Eatery Blue/UI/EateryScreen/MenuHeaderView.swiftEatery Blue/UI/HomeScreen/HomeViewController.swift
Total is 108
MrPeterss
approved these changes
Apr 19, 2026
Contributor
MrPeterss
left a comment
MrPeterss
left a comment
There was a problem hiding this comment.
Nice work! Looks great, Amy!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
Fixes 3 UI issues relating to padding and constraints in
MenuHeaderViewandHomeViewController.Changes Made
Change 1: Increased bottom inset of the collectionView of Eatery cards in HomeViewController
collectionView.contentInset.bottomso that the collectionView ends aligned with theCompareMenusButtonbottom inset (108 above the bottom of the screen).CompareMenusButton.Change 2: "Change Date" button internal spacing fixes and padding adjustments in MenuHeaderView
.lessThanOrEqualTo().Change 3: Prevent overlap with the day title
lessThanOrEqualTo()in order to prevent overlap with the "Change Date" button.Test Coverage
Screenshots
Before fixes:
Summary by CodeRabbit