Home Screen Redirection to Cycle Tab Implemented#148
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
JonahSutch
left a comment
JonahSutch
left a comment
There was a problem hiding this comment.
Requested changes. Also consider running linting before pushing.
| phaseButtonText: { | ||
| fontSize: 18, | ||
| fontWeight: "600", | ||
| color: "#fff", |
There was a problem hiding this comment.
hard coded colors. Switch to use primary color references.
| phaseButton: { | ||
| marginHorizontal: 16, | ||
| marginTop: 48, | ||
| marginBottom: -16, |
There was a problem hiding this comment.
stacking negative margins can cause overlapping issues. Consider using padding instead.
| const phaseName = currentPhase ? currentPhase.name : null; | ||
|
|
||
| const handlePhasePress = useCallback(() => { | ||
| try { |
There was a problem hiding this comment.
try - catch not needed. This will never return an error.
| <FlowChart /> | ||
| <View style={{ alignItems: "center", marginTop: 8 }}> | ||
| {/* Current Phase Button */} | ||
| {phaseName && ( |
There was a problem hiding this comment.
Not a huge issue but there is no fallback for is phaseName is still loading and returns null. You could consider adding a placeholder in case of this happening.
hauolinalani
left a comment
hauolinalani
left a comment
There was a problem hiding this comment.
Jonahs changes and then should be good
…laceholder, teammate feedback Made-with: Cursor
Made-with: Cursor
…e endings Made-with: Cursor
…urAnchor and generatePredictions Made-with: Cursor
3 participants
Build Reminder (for reviewers)
This PR requires a mobile build for both platforms
app.jsonversion has been updated and matches versioning formatActions->Manual EAS Expo Build->Run workflowdropdown -> Choose appropriate platform(s) ->Run workflow