All Swift from the ingredient list view down #49
Conversation
This file contains 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
* General project changes:
* Changed the target iOS version to 16. Our analytics shows that the
oldest version of iOS used by our users in the recent past is 16.2.
* Our project seemed to have no consistent structure with regards to
XCode groups and directory structure. I aligned both XCode groups
and directory structure with the following:
- UI: layout of views.
- Design Language: the design conventions used throughout
the app. If we ever want to reskin the app, it should be possible
to do so by merely changing things inside this folder.
- Model: describes the data that powers the app.
- Misc Utils: utilities used broadly throughout the app, such as
facilities for assertion checking.
Both Swift and ObjC sourcecode lives in this hierarchy, but I put
ObjC code within groups/directories named `ObjC` within the
hierarchy.
* The old UIKit-based `CSIngredientListVC` written in ObjC has been
replaced with SwiftUI-based `IngredientListView`. Although our usecase
is seemingly basic, I've found it difficult to do _exactly_ what we
want. I assume this might be much easier for you guys, so I sprinkled
a bunch of O&A questions in there. Almost everything downstream of
IngredientListView is now in Swift. The sole exception that I can
think of is CSGlassView.
* The model has almost fully migrated to Swift. Structs are used to
represent ingredient groups, ingredients and other aspects of the
model. IngredientsStore is the sole class in the data model and
represents the entrypoint into the persistence layer. A couple of ObjC
model classes remain to support some legacy ObjC UI code:
* `CSIngredient` - `Ingredient` struct is bridged to `CSIngredient` by
use of a dictionary.
* `CSUnit` - used across both ObjC and Swift code.
* Colors have moved into our assetcatalog for smoother support of dark
mode or other reskinning. `CSColor` enum was created to facilitate
access to the assetcatalog. Eventually, we should get to the point
where colors are accessed exclusively through `CSColor`.
* All text styling has been centralized under the `CSTextStyle` struct.
Facilities have been introduced to apply `CSTextStyle` to SwiftUI's
`Text` as well as UIKit's `UILabel`, `UIButton` and `UITextField`.
There is no styling of text outside of `CSTextStyle`, though this
isn't currently automatically enforced.
vovagalchenko
force-pushed
the
swiftui_ingredient_list
branch
from
24f0113 to
2d0b579
Compare
olgagalchenko
approved these changes
Feb 16, 2024
| case accent = "AccentColor" | ||
| case subheadingText = "SubheadingTextColor" | ||
| case contentText = "ContentTextColor" | ||
| case clear = "Clear" |
There was a problem hiding this comment.
do these reference the names in xcasset catalog?
|
|
||
| // TODO: There should be no reference to any color except through this enum | ||
|
|
||
| enum CSColor: String { |
There was a problem hiding this comment.
wait why are there 2 CSColor enums?
| struct IngredientRef { | ||
| let storedIngredientGroupIndex: Int | ||
| let ingredientIndex: Int | ||
| } |
There was a problem hiding this comment.
wow all this to add your own ingredients? do we have logging for how many people actually use that feature? idk how useful it is.
| } | ||
| } | ||
| .listStyle(PlainListStyle()) | ||
| // O&A Question: There's apparently no way to customize the appearance of the search bar... what do you suggest? |
There was a problem hiding this comment.
Hm I've never used ".searchable". We could just add our own search bar I guess. Are we just trying to style it like the rest of the app?
| // Even with this, the tap hit box for editing ingredients is still not perfect :( | ||
| // Also, it turns out that for some reason long-tapping anywhere in the cell triggers this action :( | ||
| .overlay( | ||
| NavigationLink( |
There was a problem hiding this comment.
idk, again, I've never used NavigationLink. All the navigation I've done has been custom Airbnb stuff, so idk what we have out of the box.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
ObjCwithin the hierarchy. The top-level groups/directories are:CSIngredientListVCwritten in ObjC has been replaced with SwiftUI-basedIngredientListView. Although our usecase is seemingly basic, I've found it difficult to do exactly what we want. I assume this might be much easier for you guys, so I sprinkled a bunch of O&A questions in there. Almost everything downstream of IngredientListView is now in Swift. The sole exception that I can think of is CSGlassView.CSIngredient-Ingredientstruct is bridged toCSIngredientby use of a dictionary.CSUnit- used across both ObjC and Swift code.CSColorenum was created to facilitate access to the assetcatalog. Eventually, we should get to the point where colors are accessed exclusively throughCSColor.CSTextStylestruct. Facilities have been introduced to applyCSTextStyleto SwiftUI'sTextas well as UIKit'sUILabel,UIButtonandUITextField. There is no styling of text outside ofCSTextStyle, though this isn't currently automatically enforced.