Migrate NativeWind styling system and harden calendar sync#30
Migrate NativeWind styling system and harden calendar sync#30DerpcatMusic wants to merge 45 commits intomasterfrom
Conversation
…n global.css - Rewrite global.css with full @theme token set (spacing, radius, colors) - Add dark mode via prefers-color-scheme media query - Add missing spacing tokens (componentPadding, iconContainer, iconContainerLarge, haloSize, mapMinHeight, multilineInputMinHeight) - Add radius-circle and radius-pill - Add all semantic color tokens (primary, success, danger, warning, surfaces, borders, text) - Fix kit components: kit-button-group, kit-chip, kit-disclosure-button-group, kit-segmented-toggle, kit-status-badge, kit-text-field, kit-surface - Fix payments.tsx: bulk normalize vars() ColorValue → String() - Fix sign-in-screen.tsx: remove arbitrary gap-[10px] spacing values - Fix calendar-settings.tsx: contentContainerStyle string → object - Fix modal.tsx and _layout.tsx: tokenized spacing
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4d4f118c38
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return vars({ | ||
| "--tw-bg-primary": String(palette.onPrimary), | ||
| "--tw-text-primary": String(palette.onPrimary), | ||
| minHeight: BrandSpacing.xxl + BrandSpacing.xl, | ||
| opacity: withdrawBusy ? 0.5 : isDisabled ? 0.1 : 0.25, |
There was a problem hiding this comment.
Replace
vars() with real styles for payout buttons
vars() only injects CSS custom properties, but this callback also tries to set ordinary React Native styles (minHeight, opacity) and the --tw-* values are never consumed by any bg-*/text-* class on these buttons. On the instructor payments screen, the withdraw/manage-bank CTAs therefore fall back to transparent/default styling, lose their intended height, and render low-contrast text, which makes the primary payout actions much harder to use.
Useful? React with 👍 / 👎.
| style={vars({ | ||
| backgroundColor: String(palette.dangerSubtle), | ||
| borderColor: String(palette.danger), | ||
| borderWidth: 1, | ||
| })} |
There was a problem hiding this comment.
Style the Google reconnect banners without
vars()
vars() is being used as if it were a normal style object here, so backgroundColor, borderColor, and borderWidth are ignored. In the studio calendar settings flow, the new lastError / reconnect states therefore render as plain text on the page background instead of warning cards, which makes a broken Google Calendar connection easy to miss right when we need the user to reconnect it.
Useful? React with 👍 / 👎.
Code Review Roast 🔥Verdict: 0 Issues Found | Recommendation: Merge All 3 issues from the previous review have been resolved in this commit:
Oh wait, this PR is actually clean. I need to sit down. I had my flamethrower warmed up and everything. 📊 Overall: Like finding a unicorn in production — the three issues from last time are all squashed, and no new ones crept in. Merge with confidence. Reviewed by minimax-m2.7 · incremental from 6c89c18 → HEAD Reviewed by minimax-m2.7 · 69,562 tokens |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8ee60eece8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| if (!isMapBodyReady || remoteZones === undefined) { | ||
| if (!isMapBodyReady) { |
There was a problem hiding this comment.
Wait for remote zones before mounting the map
On slow networks this branch now renders the editable map before getMyInstructorZones has resolved. useMapTabController still initializes selectedZoneIds to [] and then replaces them from remoteZones in its useEffect, so any coverage edits the instructor makes before the query finishes can be silently overwritten by the server state (or saved against an empty baseline). The previous remoteZones === undefined guard was what prevented that race.
Useful? React with 👍 / 👎.
| vars({ | ||
| "--tw-bg-payments-accent": String(palette.payments.accent), | ||
| minHeight: BrandSpacing.xxl + BrandSpacing.md, | ||
| opacity: preferenceBusy ? 0.6 : 1, | ||
| }) |
There was a problem hiding this comment.
Stop using
vars() for scheduled payout actions
When an instructor switches the payout preference to scheduled_date, this new button style passes ordinary React Native props (minHeight, opacity) through vars() and only defines --tw-bg-payments-accent without any bg-* class to consume it. In practice the Save/Cancel actions in this flow render at their default height and the primary save button loses its accent fill, so the scheduled-payout confirmation path looks disabled and is much harder to use.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 109059cc58
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const handleExternalCalendarToggle = useCallback(() => { | ||
| listRef.current?.prepareForLayoutAnimationRender?.(); | ||
| LayoutAnimation.configureNext(LayoutAnimation.Presets.easeInEaseOut); |
There was a problem hiding this comment.
Re-enable Android LayoutAnimation before toggling filters
LayoutAnimation.configureNext(...) is still called here, but the Android-only UIManager.setLayoutAnimationEnabledExperimental?.(true) setup that used to precede it was removed in this patch. On React Native Android, that flag is still required for LayoutAnimation to run, so expanding/collapsing the external-calendar section regresses to an abrupt state change on Android while iOS keeps the animation.
Useful? React with 👍 / 👎.
| const canUseGoogleCalendar = Boolean( | ||
| role && googleStatus?.connected === true && googleStatus?.hasRefreshToken === true, | ||
| ); | ||
| const canShowGoogleAgenda = canUseGoogleCalendar; |
There was a problem hiding this comment.
Continue showing cached Google events during reconnect state
This now treats hasRefreshToken === false as “cannot show Google agenda”, so the tab skips getMyGoogleCalendarAgenda entirely for users whose Google connection needs a reconnect. The backend still returns previously imported calendarExternalEvents for any integration with status === "connected", so affected instructors/studios suddenly see an empty calendar even though we still have usable cached events; the hardening only needed to stop sync, not hide existing rows.
Useful? React with 👍 / 👎.
| const eligibility = await loadInstructorEligibility(ctx, instructor._id); | ||
| if (eligibility.coverageCount === 0) { | ||
| return []; | ||
| } | ||
|
|
There was a problem hiding this comment.
Query studio markers from the draft map selection
The new marker query is seeded only from loadInstructorEligibility(...), i.e. the instructor’s persisted coverage. In the map tab we then filter that fixed result by selectedZoneIds, so when an instructor adds a new zone while editing coverage, that zone can never show any studio markers until after they save and refetch. That makes the new studio-discovery flow look broken հենց when users are exploring new areas.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d7bec3dc2e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const statusTone = getJobStatusToneWithReason(row.jobStatus, row.closureReason); | ||
| const statusLabel = t(getJobStatusTranslationKey(row.jobStatus, row.closureReason)); |
There was a problem hiding this comment.
Derive archive chip status from application outcome
The archive includes entries where the application itself is terminal (rejected/withdrawn in instructor-feed.tsx), but this row computes its chip label and tone strictly from row.jobStatus. In that scenario, a rejected application against a still-open job is shown as “Open”, which misreports the instructor’s actual outcome and makes the archive unreliable for status tracking. The chip should prioritize row.applicationStatus when it is terminal, or otherwise use a combined precedence rule.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aa8e5758a6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| vars({ | ||
| "--tw-bg-primary": String(palette.didit.accent), | ||
| "--tw-text": String(palette.onPrimary), | ||
| opacity: pressed ? 0.85 : 1, |
There was a problem hiding this comment.
Replace vars-based CTA styling in verify-bank modal
The identity-verification CTA in the “verify to connect bank” modal is styled via vars({ "--tw-bg-primary": ..., "--tw-text": ... }), but this button has no bg-*/text-* utility consuming those variables, so the intended primary colors are not applied and the action renders like an unstyled secondary control. In the unverified instructor payout flow this is the critical next step, so losing primary-action styling makes the KYC path easy to miss and hurts completion.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🔥 The Roast: The imageSize property was removed from getStudioMarkerMetrics, but now logos fill 100% of the marker circle instead of ~75-85%. If a studio has a rectangular logo (which they do), it gets brutally cropped into a circle — like forcing a wide-screen movie into a 1:1 aspect ratio. Your favorite band's album art deserves better.
🩹 The Fix:
Restore the imageSize property in getStudioMarkerMetrics and use it for the Image style:
width: imageSize,
height: imageSize,
borderRadius: imageSize / 2,
📏 Severity: warning
| <Image | ||
| source={studio.logoImageUrl as string} | ||
| style={{ | ||
| width: markerSize, |
There was a problem hiding this comment.
🔥 The Roast: The imageSize property was removed from getStudioMarkerMetrics, but now logos fill 100% of the marker circle instead of ~75-85%. If a studio has a rectangular logo (which they do), it gets brutally cropped into a circle — like forcing a wide-screen movie into a 1:1 aspect ratio. Your favorite band's album art deserves better.
🩹 The Fix:
Restore the imageSize property in getStudioMarkerMetrics and use it for the Image style:
| width: markerSize, | |
| width: imageSize, | |
| height: imageSize, | |
| borderRadius: imageSize / 2, |
📏 Severity: warning
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e7f130a6d0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const overlayBottom = Math.max(safeBottom, 16) + 16; | ||
| // Native tabs already account for bottom safe area. Floating controls should use the same | ||
| // semantic gutter on both axes instead of double-counting the bottom inset. | ||
| const overlayBottom = BrandSpacing.lg; |
There was a problem hiding this comment.
Include safe inset in overlayBottom spacing
Changing overlayBottom to a fixed BrandSpacing.lg drops bottom-safe-area compensation, and I confirmed this value is used by bottom-anchored UI in TabOverlayAnchor and profile footer actions (for example, instructor location/sports screens). On devices with a non-zero home-indicator inset, those floating/absolute controls can render into the unsafe region and become partially obscured or harder to tap; the previous calculation avoided that by incorporating safeBottom.
Useful? React with 👍 / 👎.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
…tion UI
Schema
- Added addressCity, addressStreet, addressNumber, addressFloor, addressPostalCode to instructorProfiles and studioProfiles tables (convex/schema.ts)
Convex backend
- Updated getMyInstructorSettings / updateMyInstructorSettings to read/write all 5 new structured fields (convex/users.ts)
- Updated getMyStudioSettings / updateMyStudioSettings to read/write all 5 new structured fields (convex/users.ts)
OSM address parsing (src/lib/google-places.ts)
- Extended PlaceCoordinates type with city?, street?, streetNumber?, postalCode?
- Rewrote fetchOsmAutocomplete to parse OSM addressdetails, extract structured parts, cache them, and return cleaner mainText (street+number) and secondaryText (city, postal code)
- Added fetchPlaceByZipCode() for Israeli postal code lookup via OSM Nominatim (countrycodes=il)
Location resolution (src/lib/location-zone.ts)
- Extended ResolvedLocation type with structured address fields
- Rewrote reverseGeocodeOnWeb and geocodeAddressOnWeb to extract and return structured address data from OSM
- Simplified formatAddress — outputs only "{streetNumber} {street} | {city}, {postalCode}", removed subregion/region noise
- Updated resolveCoordinatesToZone and resolveAddressToZone to populate structured fields
UI (src/app/(app)/(instructor-tabs)/instructor/profile/location.tsx)
- Rewrote with distill principles: removed hero card, single search bar as primary path, auto-filled structured address summary below search bar, manual mode as quiet escape-hatch link, zip lookup as secondary action link, compact zone section with dot indicator
- Added ManualField + FieldLabel sub-components with NativeWind 5 (no hardcoded values, BrandSpacing/BrandType/palette tokens throughout)
- All new translation strings added to both en.ts and he.ts
i18n
- Added English + Hebrew translation keys: enterManually, backToSearch, findByZip, fieldCity/Street/Number/Floor/ZipCode, fieldCityPlaceholder, fieldStreetPlaceholder, zipNotFound
Lint
- Fixed import ordering in onboarding.tsx, profile-role-switcher-card.tsx, address-autocomplete.tsx, tw/index.tsx
Top sheet
- Reworked the global top sheet transition path so the sheet frame stays anchored while content animates inside it
- Switched content transitions to a lightweight fade and restored animation coverage for sticky header/footer rich-render paths
- Fixed multiple layout regressions in the transition wrapper that caused hidden content in map, home, calendar, and profile tabs
- Removed jobs-specific local header layout animation so instructor jobs uses the same global top-sheet behavior as other tabs
- Measured real profile header/subpage header content to drive collapsed sheet height instead of relying on hardcoded step guesses
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
src/components/ui/kit/kit-switch.tsx
Outdated
| const { interaction } = useKitTheme(); | ||
| const progress = useSharedValue(value ? 1 : 0); | ||
| const pressedTrackColor = value | ||
| ? (interaction.switchThumbOn as string) |
There was a problem hiding this comment.
🔥 The Roast: When the switch is ON and pressed, pressedTrackColor uses interaction.switchThumbOn — the thumb color — instead of interaction.switchTrackOn — the track color. That's like pressing a doorbell and having the house paint itself. The thumb and track are not interchangeable.
🩹 The Fix: The pressed ON state should use interaction.switchTrackOn (track color when on), not interaction.switchThumbOn (thumb color when on).
📏 Severity: warning
|
|
||
| // Gentle breathing pulse for the symbol | ||
| const pulse = useSharedValue(1); | ||
| useEffect(() => { |
There was a problem hiding this comment.
🔥 The Roast: This useEffect kicks off withRepeat(-1) — an infinite loop animation — but returns no cleanup function. When this component unmounts (navigation away, error boundary, etc.), React Native Reanimated will complain about "animation on unmounted component." Classic "I will remember to add cleanup later" energy.
🩹 The Fix:
useEffect(() => {
pulse.value = withRepeat(
withSequence(
withTiming(1.04, { duration: 1200, easing: Easing.out(Easing.exp) }),
withTiming(1, { duration: 1200, easing: Easing.in(Easing.exp) }),
),
-1,
false,
);
return () => {
pulse.value = 1;
};
}, [pulse]);📏 Severity: warning
…ept/reject - Instructor home: horizontal snap-to-page carousel for available jobs with animated dot indicators (shared JobCarouselDots component). Empty state card when no jobs are available. - Studio home: review queue carousel with per-application Accept/Reject buttons wired to reviewApplication mutation. Empty state when queue is clear. Optimistic update via Convex refetch after mutation. - JobCarouselDots: reusable animated dot component using Reanimated SharedValue + withSpring for smooth active-dot scale/opacity transitions. - i18n: new keys for empty states (instructor + studio review queues).
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
| setReviewingId(applicationId); | ||
| try { | ||
| await reviewApplication({ applicationId, status }); | ||
| } catch (_err) { |
There was a problem hiding this comment.
🔥 The Roast: Silent error handling is like a driver who nods yes but never actually brakes. The button goes into "reviewing" mode, then silently snaps back — leaving the user wondering if their "accept" actually worked.
🩹 The Fix: Add user-facing error feedback. Even a simple Alert.alert(t("common.error"), t("home.studio.reviewError")) would prevent users from thinking their tap glitched.
📏 Severity: suggestion
…p, kit-switch pressed color - studio-home-content: surface error text on card when reviewApplication mutation fails instead of silently swallowing the error - loading-screen: add cancelAnimation() cleanup to infinite pulse useEffect to prevent animation leak on unmount - kit-switch: pressedTrackColor now uses switchTrackOn (not switchThumbOn) when switch value is true + pressed
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Summary
Validation