feat: stitch-based UI rollout + customer E2E flow wiring + provider flow fix#3
Merged
Merged
Conversation
Contributor
|
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 (3)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a session context/provider and token normalization; implements sign-in and booking API actions with tests; introduces multiple Expo Router routes and React Native screens (auth, home-triage, booking-wizard, active-job, provider-profile); exports new UI design tokens; and adds static HTML prototypes plus planning/execution docs. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant AuthScreen as AuthEntryScreen
participant AuthAction as signInWithCredentials
participant API as Auth API
participant SessionProv as SessionProvider
participant Router as Router
User->>AuthScreen: submit email/password/role
AuthScreen->>AuthAction: onSignIn({...})
AuthAction->>API: POST /api/v1/auth/sign-in
API-->>AuthAction: { token/sessionToken, role }
AuthAction-->>AuthScreen: { ok:true, sessionToken, role }
AuthScreen->>SessionProv: setSession({ status:'authenticated', sessionToken, role })
SessionProv->>Router: navigate to /provider or /home-triage
Router-->>User: destination screen
sequenceDiagram
participant User as User
participant WizardRoute as BookingWizardRoute
participant WizardScreen as BookingWizard
participant SessionProv as SessionProvider
participant Actions as submitBooking
participant API as Bookings API
participant Router as Router
User->>WizardRoute: open /booking-wizard
WizardRoute->>SessionProv: read session, resolveSessionToken()
User->>WizardScreen: complete steps & confirm
WizardScreen->>SessionProv: resolveSessionToken()
WizardScreen->>Actions: submitBooking({..}, token)
Actions->>API: POST /api/v1/bookings (Authorization: Bearer <token>)
API-->>Actions: { bookingId, status }
Actions-->>WizardScreen: { ok:true, bookingId }
WizardScreen->>Router: replace /active-job with bookingId
Router-->>User: active job screen
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 19
🧹 Nitpick comments (14)
packages/ui/src/index.ts (1)
76-79: Consider exportingShadowtype for parity.You export types for the other token groups, but not
shadow. Adding it keeps typed API usage consistent.💡 Suggested addition
export type Colors = typeof colors; export type Spacing = typeof spacing; export type Radius = typeof radius; +export type Shadow = typeof shadow; export type Typography = typeof typography;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/index.ts` around lines 76 - 79, Add a Shadow type export for parity with the other token groups: export type Shadow = typeof shadow; — update the module's type exports (alongside Colors, Spacing, Radius, Typography) so consumers can import the Shadow type and keep the typed API consistent, referencing the existing shadow identifier.stitch-designs/html/05-provider-partner-view.html (1)
8-9: Duplicate Material Symbols font import.Same font stylesheet imported on lines 8 and 9. Remove the duplicate.
🧹 Remove duplicate import
<link href="https://fonts.googleapis.com/css2?family=Newsreader:ital,opsz,wght@0,6..72,200..800;1,6..72,200..800&family=Plus+Jakarta+Sans:wght@200..800&display=swap" rel="stylesheet"/> <link href="https://fonts.googleapis.com/css2?family=Material+Symbols+Outlined:wght,FILL@100..700,0..1&display=swap" rel="stylesheet"/> -<link href="https://fonts.googleapis.com/css2?family=Material+Symbols+Outlined:wght,FILL@100..700,0..1&display=swap" rel="stylesheet"/>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stitch-designs/html/05-provider-partner-view.html` around lines 8 - 9, Remove the duplicate Material Symbols font stylesheet import: in the HTML file there are two identical <link> tags referencing the href "https://fonts.googleapis.com/css2?family=Material+Symbols+Outlined..." (both using rel="stylesheet"); delete one of these duplicate <link> elements so the font is only imported once.stitch-designs/html/03-conversational-booking.html (1)
8-10: Duplicate Material Symbols font import.Same font stylesheet imported on lines 8 and 10. Remove the duplicate to avoid redundant network requests.
🧹 Remove duplicate import
<script src="https://cdn.tailwindcss.com?plugins=forms,container-queries"></script> <link href="https://fonts.googleapis.com/css2?family=Material+Symbols+Outlined:wght,FILL@100..700,0..1&display=swap" rel="stylesheet"/> <link href="https://fonts.googleapis.com/css2?family=Work+Sans:wght@400;500;600;700&display=swap" rel="stylesheet"/> -<link href="https://fonts.googleapis.com/css2?family=Material+Symbols+Outlined:wght,FILL@100..700,0..1&display=swap" rel="stylesheet"/>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stitch-designs/html/03-conversational-booking.html` around lines 8 - 10, Remove the duplicate Material Symbols import: delete the redundant <link> element that has href="https://fonts.googleapis.com/css2?family=Material+Symbols+Outlined:wght,FILL@100..700,0..1&display=swap" so only one <link> for the Material Symbols font remains (leave the Work Sans <link> intact); ensure the remaining <link> is the exact Material Symbols href so you avoid redundant network requests.stitch-designs/html/06-admin-dashboard.html (2)
9-10: Duplicate Material Symbols font import.The same font stylesheet is imported twice (lines 9 and 10), which causes unnecessary network requests. Remove the duplicate.
🧹 Remove duplicate import
<link href="https://fonts.googleapis.com/css2?family=Newsreader:ital,opsz,wght@0,6..72,200..800;1,6..72,200..800&family=Plus+Jakarta+Sans:wght@200..800&display=swap" rel="stylesheet"/> <link href="https://fonts.googleapis.com/css2?family=Material+Symbols+Outlined:wght,FILL@100..700,0..1&display=swap" rel="stylesheet"/> -<link href="https://fonts.googleapis.com/css2?family=Material+Symbols+Outlined:wght,FILL@100..700,0..1&display=swap" rel="stylesheet"/>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stitch-designs/html/06-admin-dashboard.html` around lines 9 - 10, Remove the duplicate Material Symbols font <link> import by keeping a single link tag referencing the href "https://fonts.googleapis.com/css2?family=Material+Symbols+Outlined:wght,FILL@100..700,0..1&display=swap" and delete the redundant second identical <link>; ensure only one import of the Material Symbols stylesheet remains in the document.
98-98: Redundant height classes on sidebar.The
<aside>element has bothh-fullandh-screenclasses. These are functionally similar in this context (h-screenis 100vh,h-fullis 100% of parent). Pick one for clarity.🧹 Remove redundant class
-<aside class="fixed left-0 top-0 h-full flex flex-col py-6 px-4 bg-stone-50 dark:bg-stone-950 h-screen w-64 border-r-0 z-50"> +<aside class="fixed left-0 top-0 flex flex-col py-6 px-4 bg-stone-50 dark:bg-stone-950 h-screen w-64 border-r-0 z-50">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stitch-designs/html/06-admin-dashboard.html` at line 98, The aside element contains redundant height utilities ("h-full" and "h-screen"); remove one to avoid duplication—update the <aside> element (the element with classes "fixed left-0 top-0 ... h-full h-screen w-64") to keep only the intended height utility (prefer keeping "h-screen" for full-viewport height or "h-full" if you rely on the parent's height) and delete the other class.apps/product-app/src/features/booking/active-job-screen.js (1)
11-11:providerNameprop is unused.
MapPeekreceivesproviderNameas a prop but never uses it. Either remove the prop or use it (e.g., in an accessibility label or tooltip).🧹 Remove unused prop
-function MapPeek({ providerName, etaMin }) { +function MapPeek({ etaMin }) {And update the call site at line 203:
- <MapPeek providerName={providerName} etaMin={etaMin} /> + <MapPeek etaMin={etaMin} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/product-app/src/features/booking/active-job-screen.js` at line 11, MapPeek currently accepts a providerName prop that is never used; either remove providerName from the MapPeek function signature and delete any prop passing at its call sites, or use providerName (for example by adding it to an accessibility label or tooltip inside MapPeek) so it’s consumed—update the MapPeek component (function MapPeek) and adjust all callers to stop passing providerName if you remove it or to rely on the prop if you implement the label/tooltip change.apps/product-app/src/shared/session-provider.js (1)
14-17: Fire-and-forget sign-out swallows all errors silently.The
.catch(() => {})pattern is acceptable for fire-and-forget, but silently swallowing network errors can make debugging difficult. Consider logging in development.🔍 Add dev-only error logging
if (token) { fetch(`${runtimeConfig.platformApiBaseUrl}/api/v1/auth/sign-out`, { method: 'POST', headers: { authorization: `Bearer ${token}` }, - }).catch(() => {}); + }).catch((err) => { + if (__DEV__) { + console.warn('Sign-out request failed (non-blocking):', err); + } + }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/product-app/src/shared/session-provider.js` around lines 14 - 17, The fire-and-forget fetch to `${runtimeConfig.platformApiBaseUrl}/api/v1/auth/sign-out` currently swallows all errors via `.catch(() => {})`; change this to report errors in non-production environments by replacing the empty catch with a handler that logs the error (including the token/runtimeConfig context if useful) when NODE_ENV or an equivalent dev flag indicates development, while still avoiding throwing in production—locate the fetch call referencing `token` and `runtimeConfig.platformApiBaseUrl` in session-provider.js and update its `.catch` handler accordingly.stitch-designs/html/07-auth-sign-in.html (1)
9-10: Duplicate Material Symbols import can be removed.Same stylesheet is loaded twice.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stitch-designs/html/07-auth-sign-in.html` around lines 9 - 10, Remove the duplicate Material Symbols stylesheet import by keeping only one of the identical <link> entries that reference "https://fonts.googleapis.com/css2?family=Material+Symbols+Outlined:wght,FILL@100..700,0..1&display=swap" in the 07-auth-sign-in.html head so the same stylesheet isn't loaded twice; locate the two identical <link> elements and delete one.apps/product-app/app/active-job.js (1)
19-24: Chat/Call actions are still stubbed.These TODO handlers are fine for now, but they should be tracked as explicit follow-up work before production rollout.
If useful, I can draft a tiny integration plan (route targets + telemetry events) for both actions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/product-app/app/active-job.js` around lines 19 - 24, The onChat and onCall handlers in active-job.js are still stubs; replace the bare TODOs in the onChat and onCall props with explicit, trackable placeholders: implement small wrapper functions (e.g., handleChatClick and handleCallClick) that (1) emit a telemetry event (e.g., trackEvent('ActiveJob.ChatClicked') / trackEvent('ActiveJob.CallInitiated')), (2) call a single centralized navigation/action entrypoint (e.g., openInAppChat or initiateCall) or dispatch a Redux/Context action, and (3) include a TODO tag with a ticket/issue reference (e.g., GH-ISSUE-xxxx) so follow-up work is tracked; update the JSX to use handleChatClick and handleCallClick instead of the inline TODO comments.stitch-designs/html/04-active-job.html (1)
8-10: Duplicate Material Symbols stylesheet import.The same Google Fonts URL is included twice, which adds avoidable request overhead.
♻️ Proposed fix
<link href="https://fonts.googleapis.com/css2?family=Material+Symbols+Outlined:wght,FILL@100..700,0..1&display=swap" rel="stylesheet"/> <link href="https://fonts.googleapis.com/css2?family=Work+Sans:wght@400;500;600;700&display=swap" rel="stylesheet"/> -<link href="https://fonts.googleapis.com/css2?family=Material+Symbols+Outlined:wght,FILL@100..700,0..1&display=swap" rel="stylesheet"/>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stitch-designs/html/04-active-job.html` around lines 8 - 10, Remove the duplicate Google Fonts import for the Material Symbols stylesheet: in stitch-designs/html/04-active-job.html there are two identical <link> tags with href "https://fonts.googleapis.com/css2?family=Material+Symbols+Outlined:wght,FILL@100..700,0..1&display=swap"; keep a single <link> for that href (or deduplicate by merging with the other font links) to eliminate the redundant request.stitch-designs/html/01-home-triage.html (2)
8-10: Deduplicate Material Symbols import.Line 8 and Line 10 load the same stylesheet; remove one include.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stitch-designs/html/01-home-triage.html` around lines 8 - 10, Remove the duplicate Material Symbols stylesheet link: there are two identical <link> elements referencing "https://fonts.googleapis.com/css2?family=Material+Symbols+Outlined:wght,FILL@100..700,0..1&display=swap"; keep a single <link> with that href and remove the other one so the Material Symbols import is only included once (leave the Work Sans link intact).
89-92: Keep triage counts aligned with app constants to preserve design↔app parity.Line 91 (
12 Pros nearby) and Line 106 (8 Pros nearby) diverge fromapps/product-app/src/features/marketplace/home-triage-screen.jsvalues (4 and 2). Keeping these synchronized avoids confusion during QA and handoff.Also applies to: 104-107
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stitch-designs/html/01-home-triage.html` around lines 89 - 92, The hard-coded triage counts in the HTML ("12 Pros nearby" and "8 Pros nearby" inside the div with class "pulse-dot" / "flex items-center...") must be updated to match the app constants used in marketplace/home-triage-screen.js (which currently render 4 and 2); replace those literal strings so the design markup reflects the same counts as the source of truth in home-triage-screen.js to maintain parity between design and app.apps/product-app/src/features/auth/auth-entry-screen.js (1)
97-101: Disable Sign In when credentials are blank.Line 97 currently allows submitting empty inputs, which generates unnecessary auth calls and weakens UX feedback.
Proposed fix
const handleSignIn = () => { - if (onSignIn) { + if (!email.trim() || !password) return; + if (onSignIn) { onSignIn({ email, password, role }); } }; @@ <TouchableOpacity @@ + disabled={!email.trim() || !password} + accessibilityState={{ disabled: !email.trim() || !password }} style={{ - backgroundColor: colors.primary, + backgroundColor: !email.trim() || !password ? colors.muted : colors.primary,Also applies to: 221-236
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/product-app/src/features/auth/auth-entry-screen.js` around lines 97 - 101, handleSignIn currently calls onSignIn even when email/password are empty; update the handler so it trims inputs and returns early (no call to onSignIn) when email or password are blank, and ensure the sign-in button is disabled when credentials are empty; apply the same change to the analogous handler in the 221-236 range (the signup/auth submission handler) so both submission functions validate trimmed email and password and prevent calling onSignIn/onSignUp with empty values.stitch-designs/html/02-provider-profile.html (1)
11-12: Remove duplicate Material Symbols stylesheet include.Lines 11 and 12 are identical imports; keep one to avoid redundant network work.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stitch-designs/html/02-provider-profile.html` around lines 11 - 12, There are two identical Material Symbols stylesheet <link> tags duplicated in the HTML head; remove one of the duplicate <link href="https://fonts.googleapis.com/css2?family=Material+Symbols+Outlined:wght,FILL@100..700,0..1&display=swap" rel="stylesheet"/> entries so only a single include remains (locate the duplicate lines in the file containing the Material Symbols <link> entries and delete the redundant one).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/admin-web/next-env.d.ts`:
- Line 3: The current next-env.d.ts contains a brittle import of
"./.next/dev/types/routes.d.ts"; remove this import from next-env.d.ts and
instead add the produced type files to your TypeScript project include settings
(e.g., add ".next/types/**/*.ts" to the "include" array in tsconfig.json) so
Next's generated route types are picked up without requiring a direct import of
the .next folder; update any CI/build docs if they relied on that import so the
.next generation step runs before type-checking.
In `@apps/product-app/app/active-job.js`:
- Around line 11-13: The numeric parsing for etaMin and delayMin can yield NaN
when params are malformed; update the parsing logic in active-job.js so you
coerce params.etaMin and params.delayMin to numbers then check Number.isNaN and
fall back to the default values (12 for etaMin, 5 for delayMin). Locate the
object properties etaMin and delayMin and replace the current inline Number(...)
expressions with guarded parsing (parse/Number conversion + Number.isNaN check)
to ensure the UI never receives NaN.
In `@apps/product-app/app/auth.js`:
- Around line 16-36: The sign-in flow in handleSignIn can leave loading stuck
true if signInWithCredentials throws; wrap the awaited call and subsequent logic
in a try/finally so setLoading(false) always runs. Specifically, in handleSignIn
around signInWithCredentials(...) ensure you call setLoading(true) before the
try, move the await and the result handling (setError, setSession,
router.replace) into the try block, and call setLoading(false) in the finally
block; keep setError(null) at the start and preserve use of
signInWithCredentials, setError, setSession, and router.replace.
- Line 40: AuthEntryScreen's create-account button is not wired because only
onSignIn is passed; add and pass an onCreateAccount prop by implementing a
handler (e.g., handleCreateAccount) in the same module and pass it alongside
handleSignIn when rendering AuthEntryScreen. Update the JSX to <AuthEntryScreen
onSignIn={handleSignIn} onCreateAccount={handleCreateAccount} />, implement
handleCreateAccount to perform the existing sign-up flow (navigation or API
call) consistent with handleSignIn, and ensure any required imports or state
used by the sign-up flow are added or reused.
In `@apps/product-app/app/booking-wizard.js`:
- Around line 17-31: handleComplete currently allows unauthenticated submissions
(token null) and doesn't reset loading if submitBooking throws; update
handleComplete to first guard against unauthenticated users by returning early
and setting an appropriate error when session.status !== 'authenticated' (use
session and params.category to locate logic), then wrap the submit flow in
try/catch/finally around the async call to submitBooking so any thrown error
sets setError(...) and the finally block always calls setLoading(false); keep
the initial loading guard (loading) and ensure setError(null) is still called
before attempting the submit.
In `@apps/product-app/app/home-triage.js`:
- Around line 13-15: The onChangeAddress handler in HomeTriage is currently a
no-op; replace the TODO with a minimal, functional flow that opens a temporary
location-picker UI (e.g., navigate to a "LocationPicker" route or show a modal)
and returns a selected address back to HomeTriage; wire the opened picker to
call a callback like onAddressSelected or updateAddress (or set local state)
when the user picks a location so tapping the control produces visible behavior
instead of doing nothing. Ensure you modify the onChangeAddress implementation
to invoke the app's navigation/modal API (navigation.navigate('LocationPicker')
or openModal('LocationPicker')) and handle the result to update the address
field in HomeTriage.
In `@apps/product-app/src/features/booking/active-job-screen.js`:
- Around line 27-39: The dashed border View with inline style containing
borderStyle: 'dashed', borderWidth: 1, and borderColor: colors.primary should
include a non-zero borderRadius to render dashed borders on Android; update the
View (the element with that style block) to add borderRadius: 1 (or similar
small value) alongside the other style props so the dashed border displays
correctly on Android.
In `@apps/product-app/src/features/booking/booking-wizard-actions.js`:
- Around line 3-15: In submitBooking, validate the token before making the
fetch: check the token parameter (and any session fallback if applicable) and
throw or return a clear error (e.g., "Missing authentication token") when
undefined/empty instead of calling fetch and sending "Bearer undefined"; update
the start of the submitBooking function to perform this guard (referencing
submitBooking and token) so the request is not attempted without valid
credentials and callers get an explicit error.
In `@apps/product-app/src/features/booking/booking-wizard-screen.js`:
- Around line 6-31: The STEPS constant always contains plumbing-specific
questions/options which ignores the component prop category and causes wrong
UX/metadata; replace the static STEPS with a category-aware step builder (e.g.,
implement getStepsForCategory(category) or computeSteps(category)) and use that
in the BookingWizardScreen render logic so the question text, subtitle and
options vary for electrical vs plumbing flows; update all places that referenced
STEPS (including where options are rendered) to call the new function or derived
steps so the correct options and copy are produced based on the category prop.
- Around line 95-103: LocationStep's Edit CTA is inert because the onEdit prop
is never forwarded from the parent — locate where LocationStep is instantiated
(the parent render around the earlier referenced lines, e.g.,
BookingWizardScreen or the booking wizard render where LocationStep is used) and
pass a real handler (e.g., handleEditLocation or openLocationEditor) instead of
leaving onEdit undefined; implement or reuse an existing method that opens the
location editor/modal (or navigates to the address edit step) and pass it as the
onEdit prop to LocationStep so the TouchableOpacity becomes functional (ensure
the handler signature matches LocationStep's onEdit and keep onConfirm/onEdit
props consistent).
In `@apps/product-app/src/features/marketplace/provider-profile-screen.js`:
- Line 293: The About {p.name.split(' ')[0]} expression assumes p.name is a
non-empty string and will throw if null/undefined; update the rendering in
provider-profile-screen.js to defensively handle missing names by using a safe
fallback (e.g., use optional chaining or nullish coalescing on p.name and a
default like '' or 'Provider'), then split that safe string and grab [0] (or
fall back to a default label) so the UI never calls .split on null/undefined.
- Around line 182-183: The component ProviderProfile currently masks missing
integration by defaulting onClose and onRequest to no-op handlers (remove the
patterns like onClose ?? (() => {}) and onRequest ?? (() => {})); instead make
these callbacks required at runtime by removing the no-op defaults in
ProviderProfile and replacing calls to onClose/onRequest with a guarded
invocation that logs/throws if the prop is absent (e.g., if (typeof onClose !==
'function') { console.error('ProviderProfile: missing onClose'); /* or throw new
Error(...) */ } else { onClose(...) }), and apply the same change to the other
occurrences noted around the sections corresponding to lines 205-209 and 334-338
so CTAs cannot silently do nothing.
In `@apps/product-app/src/features/provider/provider-screen.js`:
- Around line 28-30: The early conditional return "if (session.status !==
'authenticated') return null;" causes hooks (notably the useEffect declared
later) to be skipped on some renders and run on others, breaking React's hook
order; remove or move this early return so hooks always execute: either always
invoke React hooks (keep the useEffect and other hooks at top-level) and delay
the conditional render by checking session.status after hooks (e.g., compute
isAuthenticated and return null only after hooks run), or keep the return but
instead guard inside the useEffect (wrap its body with if (session.status !==
'authenticated') return;) so useEffect is declared unconditionally; locate the
conditional return and the useEffect in provider-screen.js and adjust
accordingly (symbols: session.status, the early return null, and the useEffect).
In `@apps/product-app/src/shared/session-provider.js`:
- Line 5: The default SessionContext value only contains status and can cause
undefined errors for callers expecting setSession or signOut; update the
createContext call for SessionContext to include safe defaults for these symbols
(e.g., setSession: () => {}, signOut: () => {} ) or functions that throw a clear
error if used outside a provider (so consumers fail fast). Locate the
SessionContext export and change createContext({ status: 'unauthenticated' }) to
include the same shape used by your SessionProvider (status, session if
applicable, setSession, signOut) with no-op or throwing implementations to
prevent runtime undefined access.
In `@packages/ui/src/index.ts`:
- Around line 63-67: The deprecated compatibility object incorrectly maps
designTokens.color.surface to colors.background; update the mapping so
designTokens.color.surface uses colors.surface instead of colors.background by
changing the assignment in the compatibility object where color.primary,
color.accent, color.surface, color.text are defined (look for the color block
that references colors.* in packages/ui/src/index.ts) so consumers relying on
designTokens get the correct surface token.
In `@stitch-designs/html/04-active-job.html`:
- Line 75: Update the SVG element with class "absolute inset-0 w-full h-full
pointer-events-none" to use the correct SVG attribute casing: change the
lowercase attributes `viewbox` to `viewBox` and `preserveaspectratio` to
`preserveAspectRatio` so browsers honor the scaling behavior; locate the <svg
...> tag in stitch-designs/html/04-active-job.html and replace those attribute
names accordingly.
In `@stitch-designs/html/05-provider-partner-view.html`:
- Line 206: The class token "cubic-bezier(0.4, 0, 0.2, 1)" on the anchor
element's class list is invalid Tailwind syntax; remove that token from the
class attribute on the <a> element (the element with classes "flex flex-col
items-center justify-center text-emerald-900 ... transition-all") or replace it
with a valid Tailwind easing utility such as "ease-in-out" or an arbitrary value
"ease-[cubic-bezier(0.4,0,0.2,1)]" if you specifically need that timing
function.
In `@stitch-designs/html/07-auth-sign-in.html`:
- Around line 95-97: The close button with the span containing the material icon
(the button element wrapping <span data-icon="close">close</span>) is missing an
accessible name; add an aria-label (e.g., aria-label="Close") to the button or
include visually-hidden text inside the button so screen readers announce its
purpose, ensuring you update the button element (the one with classes
"text-primary hover:opacity-80...") rather than the icon span.
- Around line 137-140: The email and password inputs currently rely on
placeholders and lack accessible labels; add explicit <label> elements tied to
each input by adding unique id attributes (e.g., id="email" and id="password")
on the <input> elements and corresponding <label for="email">Email
Address</label> and <label for="password">Password</label> elements; if you
don't want visible labels, use visually-hidden CSS (e.g., class="sr-only") on
the labels or add aria-label attributes matching the placeholders, ensuring the
labels reference the same id values so screen readers can identify the fields.
---
Nitpick comments:
In `@apps/product-app/app/active-job.js`:
- Around line 19-24: The onChat and onCall handlers in active-job.js are still
stubs; replace the bare TODOs in the onChat and onCall props with explicit,
trackable placeholders: implement small wrapper functions (e.g., handleChatClick
and handleCallClick) that (1) emit a telemetry event (e.g.,
trackEvent('ActiveJob.ChatClicked') / trackEvent('ActiveJob.CallInitiated')),
(2) call a single centralized navigation/action entrypoint (e.g., openInAppChat
or initiateCall) or dispatch a Redux/Context action, and (3) include a TODO tag
with a ticket/issue reference (e.g., GH-ISSUE-xxxx) so follow-up work is
tracked; update the JSX to use handleChatClick and handleCallClick instead of
the inline TODO comments.
In `@apps/product-app/src/features/auth/auth-entry-screen.js`:
- Around line 97-101: handleSignIn currently calls onSignIn even when
email/password are empty; update the handler so it trims inputs and returns
early (no call to onSignIn) when email or password are blank, and ensure the
sign-in button is disabled when credentials are empty; apply the same change to
the analogous handler in the 221-236 range (the signup/auth submission handler)
so both submission functions validate trimmed email and password and prevent
calling onSignIn/onSignUp with empty values.
In `@apps/product-app/src/features/booking/active-job-screen.js`:
- Line 11: MapPeek currently accepts a providerName prop that is never used;
either remove providerName from the MapPeek function signature and delete any
prop passing at its call sites, or use providerName (for example by adding it to
an accessibility label or tooltip inside MapPeek) so it’s consumed—update the
MapPeek component (function MapPeek) and adjust all callers to stop passing
providerName if you remove it or to rely on the prop if you implement the
label/tooltip change.
In `@apps/product-app/src/shared/session-provider.js`:
- Around line 14-17: The fire-and-forget fetch to
`${runtimeConfig.platformApiBaseUrl}/api/v1/auth/sign-out` currently swallows
all errors via `.catch(() => {})`; change this to report errors in
non-production environments by replacing the empty catch with a handler that
logs the error (including the token/runtimeConfig context if useful) when
NODE_ENV or an equivalent dev flag indicates development, while still avoiding
throwing in production—locate the fetch call referencing `token` and
`runtimeConfig.platformApiBaseUrl` in session-provider.js and update its
`.catch` handler accordingly.
In `@packages/ui/src/index.ts`:
- Around line 76-79: Add a Shadow type export for parity with the other token
groups: export type Shadow = typeof shadow; — update the module's type exports
(alongside Colors, Spacing, Radius, Typography) so consumers can import the
Shadow type and keep the typed API consistent, referencing the existing shadow
identifier.
In `@stitch-designs/html/01-home-triage.html`:
- Around line 8-10: Remove the duplicate Material Symbols stylesheet link: there
are two identical <link> elements referencing
"https://fonts.googleapis.com/css2?family=Material+Symbols+Outlined:wght,FILL@100..700,0..1&display=swap";
keep a single <link> with that href and remove the other one so the Material
Symbols import is only included once (leave the Work Sans link intact).
- Around line 89-92: The hard-coded triage counts in the HTML ("12 Pros nearby"
and "8 Pros nearby" inside the div with class "pulse-dot" / "flex
items-center...") must be updated to match the app constants used in
marketplace/home-triage-screen.js (which currently render 4 and 2); replace
those literal strings so the design markup reflects the same counts as the
source of truth in home-triage-screen.js to maintain parity between design and
app.
In `@stitch-designs/html/02-provider-profile.html`:
- Around line 11-12: There are two identical Material Symbols stylesheet <link>
tags duplicated in the HTML head; remove one of the duplicate <link
href="https://fonts.googleapis.com/css2?family=Material+Symbols+Outlined:wght,FILL@100..700,0..1&display=swap"
rel="stylesheet"/> entries so only a single include remains (locate the
duplicate lines in the file containing the Material Symbols <link> entries and
delete the redundant one).
In `@stitch-designs/html/03-conversational-booking.html`:
- Around line 8-10: Remove the duplicate Material Symbols import: delete the
redundant <link> element that has
href="https://fonts.googleapis.com/css2?family=Material+Symbols+Outlined:wght,FILL@100..700,0..1&display=swap"
so only one <link> for the Material Symbols font remains (leave the Work Sans
<link> intact); ensure the remaining <link> is the exact Material Symbols href
so you avoid redundant network requests.
In `@stitch-designs/html/04-active-job.html`:
- Around line 8-10: Remove the duplicate Google Fonts import for the Material
Symbols stylesheet: in stitch-designs/html/04-active-job.html there are two
identical <link> tags with href
"https://fonts.googleapis.com/css2?family=Material+Symbols+Outlined:wght,FILL@100..700,0..1&display=swap";
keep a single <link> for that href (or deduplicate by merging with the other
font links) to eliminate the redundant request.
In `@stitch-designs/html/05-provider-partner-view.html`:
- Around line 8-9: Remove the duplicate Material Symbols font stylesheet import:
in the HTML file there are two identical <link> tags referencing the href
"https://fonts.googleapis.com/css2?family=Material+Symbols+Outlined..." (both
using rel="stylesheet"); delete one of these duplicate <link> elements so the
font is only imported once.
In `@stitch-designs/html/06-admin-dashboard.html`:
- Around line 9-10: Remove the duplicate Material Symbols font <link> import by
keeping a single link tag referencing the href
"https://fonts.googleapis.com/css2?family=Material+Symbols+Outlined:wght,FILL@100..700,0..1&display=swap"
and delete the redundant second identical <link>; ensure only one import of the
Material Symbols stylesheet remains in the document.
- Line 98: The aside element contains redundant height utilities ("h-full" and
"h-screen"); remove one to avoid duplication—update the <aside> element (the
element with classes "fixed left-0 top-0 ... h-full h-screen w-64") to keep only
the intended height utility (prefer keeping "h-screen" for full-viewport height
or "h-full" if you rely on the parent's height) and delete the other class.
In `@stitch-designs/html/07-auth-sign-in.html`:
- Around line 9-10: Remove the duplicate Material Symbols stylesheet import by
keeping only one of the identical <link> entries that reference
"https://fonts.googleapis.com/css2?family=Material+Symbols+Outlined:wght,FILL@100..700,0..1&display=swap"
in the 07-auth-sign-in.html head so the same stylesheet isn't loaded twice;
locate the two identical <link> elements and delete one.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6db60f53-06b4-48dd-a946-e886a5341d50
⛔ Files ignored due to path filters (7)
stitch-designs/screenshots/01-home-triage.pngis excluded by!**/*.pngstitch-designs/screenshots/02-provider-profile.pngis excluded by!**/*.pngstitch-designs/screenshots/03-conversational-booking.pngis excluded by!**/*.pngstitch-designs/screenshots/04-active-job.pngis excluded by!**/*.pngstitch-designs/screenshots/05-provider-partner-view.pngis excluded by!**/*.pngstitch-designs/screenshots/06-admin-dashboard.pngis excluded by!**/*.pngstitch-designs/screenshots/07-auth-sign-in.pngis excluded by!**/*.png
📒 Files selected for processing (27)
apps/admin-web/next-env.d.tsapps/admin-web/package.jsonapps/product-app/app/_layout.jsapps/product-app/app/active-job.jsapps/product-app/app/auth.jsapps/product-app/app/booking-wizard.jsapps/product-app/app/home-triage.jsapps/product-app/app/index.jsapps/product-app/app/provider-profile.jsapps/product-app/src/features/auth/auth-entry-actions.jsapps/product-app/src/features/auth/auth-entry-screen.jsapps/product-app/src/features/booking/active-job-screen.jsapps/product-app/src/features/booking/booking-screen.jsapps/product-app/src/features/booking/booking-wizard-actions.jsapps/product-app/src/features/booking/booking-wizard-screen.jsapps/product-app/src/features/marketplace/home-triage-screen.jsapps/product-app/src/features/marketplace/provider-profile-screen.jsapps/product-app/src/features/provider/provider-screen.jsapps/product-app/src/shared/session-provider.jspackages/ui/src/index.tsstitch-designs/html/01-home-triage.htmlstitch-designs/html/02-provider-profile.htmlstitch-designs/html/03-conversational-booking.htmlstitch-designs/html/04-active-job.htmlstitch-designs/html/05-provider-partner-view.htmlstitch-designs/html/06-admin-dashboard.htmlstitch-designs/html/07-auth-sign-in.html
Comment on lines
+17
to
+31
| const handleComplete = async ({ issueType, urgency, address }) => { | ||
| if (loading) return; | ||
| const token = session.status === 'authenticated' ? session.token : null; | ||
| setLoading(true); | ||
| setError(null); | ||
|
|
||
| const result = await submitBooking( | ||
| { issueType, urgency, address, category: params.category }, | ||
| token, | ||
| ); | ||
| setLoading(false); | ||
|
|
||
| if (!result.ok) { | ||
| setError(result.error); | ||
| return; |
Contributor
There was a problem hiding this comment.
Guard unauthenticated submits and make loading reset exception-safe.
Line 19 still allows unauthenticated submissions (null token), and Lines 23–27/29–31 don’t handle thrown errors safely. This can trigger avoidable 401s and leave the screen stuck in loading.
Proposed fix
const handleComplete = async ({ issueType, urgency, address }) => {
if (loading) return;
- const token = session.status === 'authenticated' ? session.token : null;
+ if (session.status !== 'authenticated') {
+ router.replace('/auth');
+ return;
+ }
+ const token = session.token;
setLoading(true);
setError(null);
-
- const result = await submitBooking(
- { issueType, urgency, address, category: params.category },
- token,
- );
- setLoading(false);
-
- if (!result.ok) {
- setError(result.error);
- return;
- }
+ try {
+ const result = await submitBooking(
+ { issueType, urgency, address, category: params.category },
+ token,
+ );
+ if (!result.ok) {
+ setError(result.error);
+ return;
+ }
- router.replace({
- pathname: '/active-job',
- params: { bookingId: result.bookingId, providerName: 'Finding your pro...' },
- });
+ router.replace({
+ pathname: '/active-job',
+ params: { bookingId: result.bookingId, providerName: 'Finding your pro...' },
+ });
+ } catch (error) {
+ setError(error instanceof Error ? error.message : 'Unexpected booking failure.');
+ } finally {
+ setLoading(false);
+ }
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/product-app/app/booking-wizard.js` around lines 17 - 31, handleComplete
currently allows unauthenticated submissions (token null) and doesn't reset
loading if submitBooking throws; update handleComplete to first guard against
unauthenticated users by returning early and setting an appropriate error when
session.status !== 'authenticated' (use session and params.category to locate
logic), then wrap the submit flow in try/catch/finally around the async call to
submitBooking so any thrown error sets setError(...) and the finally block
always calls setLoading(false); keep the initial loading guard (loading) and
ensure setError(null) is still called before attempting the submit.
| <div class="absolute inset-0 bg-cover bg-center opacity-80" data-alt="Muted greyscale city map view of Vienna streets" style="background-image: url('https://lh3.googleusercontent.com/aida-public/AB6AXuBqb9l0iWXtFnS8RYAdDJIDvhpXn8wCiZztMLnex1WZe1foaGYx3hQ5lK_SHZXRty9I6IMLeRm6-f5TZ_c_E9ASrA2uWKWF40U7gMyVIeiQBrrXnalHmFEw0gOnIxvcBTD19o2HjFLTl83rBAj1VKiXhfBJGXRG60NNCOcOSFfSuR3d127x0NI3TVTV9SswhXuMwffqPa6Yv1qbCtE1l3rDPfR0cXOUdxYW0hMP9A4scJLoUQFycKPKCs3usvlKbSVPjbaHPdi_h7-m');"> | ||
| </div> | ||
| <!-- Map Route Overlay (Decorative) --> | ||
| <svg class="absolute inset-0 w-full h-full pointer-events-none" preserveaspectratio="xMidYMid slice" viewbox="0 0 400 300"> |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In inline SVG embedded in HTML, are viewBoxandpreserveAspectRatiocase-sensitive, and is lowercaseviewbox/preserveaspectratio ignored?
💡 Result:
In inline SVG embedded in HTML, viewBox and preserveAspectRatio attributes are case-sensitive according to SVG specifications. Lowercase variants like viewbox or preserveaspectratio are ignored by browsers and do not take effect. Browsers parse inline SVG as "foreign content" in HTML parsing mode. While HTML attribute names are ASCII case-insensitive (folded to lowercase for matching), SVG attributes must match the exact case defined in SVG specs (camelCase). HTML parsers preserve the original case of SVG attributes during serialization but only recognize them if they match the expected casing after case-folding for lookup. Multiple sources confirm lowercase SVG attributes fail: - SVGO and other optimizers remove unrecognized lowercase viewbox. - Testing shows renders without scaling; viewBox works. - Framework issues (Svelte, SolidJS, Stencil) lowercase them incorrectly, breaking functionality. - Tools like tidy-html5 lowercase them, causing issues unless XML mode used. Official docs (MDN, W3C SVG 2) always use exact casing: viewBox="min-x min-y width height" and preserveAspectRatio="align [meetOrSlice]". Always use precise casing: viewBox and preserveAspectRatio. Lowercase is ignored, leading to silent failures like no scaling/panning.
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/API/SVGSVGElement/viewBox
- 2: https://developer.mozilla.org/en-US/docs/Web/SVG/Reference/Attribute/preserveAspectRatio
- 3: https://www.w3.org/TR/SVG/struct.html
- 4: https://developer.mozilla.org/en-US/docs/Web/SVG/Reference/Element/svg
- 5: Attributes/Tags of an inline SVG should be case sensitive fb55/htmlparser2#539
- 6: https://developer.mozilla.org/en-US/docs/Web/SVG/Reference/Attribute/viewBox
- 7: https://dev.w3.org/SVG/proposals/svg-html/svg-html-proposal.html
- 8: SVG attribute stdDeviation changes to lowercase when used with a reactive variable solidjs/solid#2110
- 9: https://dbushell.com/2016/03/01/be-careful-with-your-viewbox
- 10: https://stackoverflow.com/questions/40919369/how-to-resize-svg-used-in-img-tag-in-html
- 11: https://www.princexml.com/forum/topic/1623/svg-viewbox-directive-ignored-in-html-mode
- 12: bug: SVG's viewBox is set to lowercase (again) stenciljs/core#3717
Fix SVG attribute casing: viewBox and preserveAspectRatio must use correct case.
Line 75 uses lowercase viewbox and preserveaspectratio, which browsers ignore per SVG specifications. This breaks the overlay scaling behavior. Update both attributes to camelCase: viewBox and preserveAspectRatio.
🔧 Proposed fix
-<svg class="absolute inset-0 w-full h-full pointer-events-none" preserveaspectratio="xMidYMid slice" viewbox="0 0 400 300">
+<svg class="absolute inset-0 w-full h-full pointer-events-none" preserveAspectRatio="xMidYMid slice" viewBox="0 0 400 300">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <svg class="absolute inset-0 w-full h-full pointer-events-none" preserveaspectratio="xMidYMid slice" viewbox="0 0 400 300"> | |
| <svg class="absolute inset-0 w-full h-full pointer-events-none" preserveAspectRatio="xMidYMid slice" viewBox="0 0 400 300"> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@stitch-designs/html/04-active-job.html` at line 75, Update the SVG element
with class "absolute inset-0 w-full h-full pointer-events-none" to use the
correct SVG attribute casing: change the lowercase attributes `viewbox` to
`viewBox` and `preserveaspectratio` to `preserveAspectRatio` so browsers honor
the scaling behavior; locate the <svg ...> tag in
stitch-designs/html/04-active-job.html and replace those attribute names
accordingly.
| <!-- BottomNavBar --> | ||
| <nav class="fixed bottom-0 left-0 w-full z-50 flex justify-around items-center px-4 pb-6 pt-3 bg-white dark:bg-stone-950 rounded-t-[32px] shadow-[0px_-4px_16px_rgba(0,0,0,0.04)] border-t border-stone-100 dark:border-stone-800"> | ||
| <!-- Requests (Active Tab) --> | ||
| <a class="flex flex-col items-center justify-center text-emerald-900 dark:text-emerald-400 bg-emerald-50 dark:bg-emerald-900/20 rounded-full px-4 py-1 transition-all cubic-bezier(0.4, 0, 0.2, 1)" href="#"> |
Contributor
There was a problem hiding this comment.
Invalid Tailwind class syntax.
cubic-bezier(0.4, 0, 0.2, 1) is a CSS timing function value, not a valid Tailwind utility class. It should be part of a transition property or removed. Tailwind's transition-all already includes a default easing.
🐛 Fix invalid class
-<a class="flex flex-col items-center justify-center text-emerald-900 dark:text-emerald-400 bg-emerald-50 dark:bg-emerald-900/20 rounded-full px-4 py-1 transition-all cubic-bezier(0.4, 0, 0.2, 1)" href="#">
+<a class="flex flex-col items-center justify-center text-emerald-900 dark:text-emerald-400 bg-emerald-50 dark:bg-emerald-900/20 rounded-full px-4 py-1 transition-all ease-in-out" href="#">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <a class="flex flex-col items-center justify-center text-emerald-900 dark:text-emerald-400 bg-emerald-50 dark:bg-emerald-900/20 rounded-full px-4 py-1 transition-all cubic-bezier(0.4, 0, 0.2, 1)" href="#"> | |
| <a class="flex flex-col items-center justify-center text-emerald-900 dark:text-emerald-400 bg-emerald-50 dark:bg-emerald-900/20 rounded-full px-4 py-1 transition-all ease-in-out" href="#"> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@stitch-designs/html/05-provider-partner-view.html` at line 206, The class
token "cubic-bezier(0.4, 0, 0.2, 1)" on the anchor element's class list is
invalid Tailwind syntax; remove that token from the class attribute on the <a>
element (the element with classes "flex flex-col items-center justify-center
text-emerald-900 ... transition-all") or replace it with a valid Tailwind easing
utility such as "ease-in-out" or an arbitrary value
"ease-[cubic-bezier(0.4,0,0.2,1)]" if you specifically need that timing
function.
Comment on lines
+95
to
+97
| <button class="text-primary hover:opacity-80 transition-opacity active:scale-95 duration-200"> | ||
| <span class="material-symbols-outlined" data-icon="close">close</span> | ||
| </button> |
Contributor
There was a problem hiding this comment.
Add an accessible name to the icon-only close button.
At Line 95, the button has no aria-label, so assistive tech won’t announce its purpose.
♿ Proposed fix
-<button class="text-primary hover:opacity-80 transition-opacity active:scale-95 duration-200">
+<button aria-label="Close sign-in" type="button" class="text-primary hover:opacity-80 transition-opacity active:scale-95 duration-200">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <button class="text-primary hover:opacity-80 transition-opacity active:scale-95 duration-200"> | |
| <span class="material-symbols-outlined" data-icon="close">close</span> | |
| </button> | |
| <button aria-label="Close sign-in" type="button" class="text-primary hover:opacity-80 transition-opacity active:scale-95 duration-200"> | |
| <span class="material-symbols-outlined" data-icon="close">close</span> | |
| </button> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@stitch-designs/html/07-auth-sign-in.html` around lines 95 - 97, The close
button with the span containing the material icon (the button element wrapping
<span data-icon="close">close</span>) is missing an accessible name; add an
aria-label (e.g., aria-label="Close") to the button or include visually-hidden
text inside the button so screen readers announce its purpose, ensuring you
update the button element (the one with classes "text-primary
hover:opacity-80...") rather than the icon span.
Comment on lines
+137
to
+140
| <input class="w-full h-16 px-6 bg-surface-container-low border-none rounded-lg focus:ring-1 focus:ring-primary focus:bg-surface-container-lowest transition-all font-body text-on-surface placeholder:text-outline/60" placeholder="Email Address" type="email"/> | ||
| </div> | ||
| <div class="relative"> | ||
| <input class="w-full h-16 px-6 bg-surface-container-low border-none rounded-lg focus:ring-1 focus:ring-primary focus:bg-surface-container-lowest transition-all font-body text-on-surface placeholder:text-outline/60" placeholder="Password" type="password"/> |
Contributor
There was a problem hiding this comment.
Form fields need explicit labels (placeholder-only is not accessible).
At Line 137 and Line 140, inputs are missing associated <label> elements.
♿ Proposed fix
<form class="w-full space-y-4 mb-8">
<div class="relative">
- <input class="w-full h-16 px-6 bg-surface-container-low border-none rounded-lg focus:ring-1 focus:ring-primary focus:bg-surface-container-lowest transition-all font-body text-on-surface placeholder:text-outline/60" placeholder="Email Address" type="email"/>
+ <label for="email" class="sr-only">Email address</label>
+ <input id="email" autocomplete="email" class="w-full h-16 px-6 bg-surface-container-low border-none rounded-lg focus:ring-1 focus:ring-primary focus:bg-surface-container-lowest transition-all font-body text-on-surface placeholder:text-outline/60" placeholder="Email Address" type="email"/>
</div>
<div class="relative">
- <input class="w-full h-16 px-6 bg-surface-container-low border-none rounded-lg focus:ring-1 focus:ring-primary focus:bg-surface-container-lowest transition-all font-body text-on-surface placeholder:text-outline/60" placeholder="Password" type="password"/>
+ <label for="password" class="sr-only">Password</label>
+ <input id="password" autocomplete="current-password" class="w-full h-16 px-6 bg-surface-container-low border-none rounded-lg focus:ring-1 focus:ring-primary focus:bg-surface-container-lowest transition-all font-body text-on-surface placeholder:text-outline/60" placeholder="Password" type="password"/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <input class="w-full h-16 px-6 bg-surface-container-low border-none rounded-lg focus:ring-1 focus:ring-primary focus:bg-surface-container-lowest transition-all font-body text-on-surface placeholder:text-outline/60" placeholder="Email Address" type="email"/> | |
| </div> | |
| <div class="relative"> | |
| <input class="w-full h-16 px-6 bg-surface-container-low border-none rounded-lg focus:ring-1 focus:ring-primary focus:bg-surface-container-lowest transition-all font-body text-on-surface placeholder:text-outline/60" placeholder="Password" type="password"/> | |
| <form class="w-full space-y-4 mb-8"> | |
| <div class="relative"> | |
| <label for="email" class="sr-only">Email address</label> | |
| <input id="email" autocomplete="email" class="w-full h-16 px-6 bg-surface-container-low border-none rounded-lg focus:ring-1 focus:ring-primary focus:bg-surface-container-lowest transition-all font-body text-on-surface placeholder:text-outline/60" placeholder="Email Address" type="email"/> | |
| </div> | |
| <div class="relative"> | |
| <label for="password" class="sr-only">Password</label> | |
| <input id="password" autocomplete="current-password" class="w-full h-16 px-6 bg-surface-container-low border-none rounded-lg focus:ring-1 focus:ring-primary focus:bg-surface-container-lowest transition-all font-body text-on-surface placeholder:text-outline/60" placeholder="Password" type="password"/> | |
| </div> | |
| </form> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@stitch-designs/html/07-auth-sign-in.html` around lines 137 - 140, The email
and password inputs currently rely on placeholders and lack accessible labels;
add explicit <label> elements tied to each input by adding unique id attributes
(e.g., id="email" and id="password") on the <input> elements and corresponding
<label for="email">Email Address</label> and <label
for="password">Password</label> elements; if you don't want visible labels, use
visually-hidden CSS (e.g., class="sr-only") on the labels or add aria-label
attributes matching the placeholders, ensuring the labels reference the same id
values so screen readers can identify the fields.
Collaborator
Author
|
🔍 Subagent analysis (2026-03-25) — CodeRabbit findings categorized. Must-Fix (15 items across 9 files) before merge:
HTML reference files have duplicate font imports (6 files) — can defer. Risk: Merging as-is introduces user-facing bugs (auth broken, booking crashes, React errors). Full report: workspace docs/status/2026-03-25-priority-portfolio-sweep.md |
…/provider flows Batch 1-3 must-fix: - sessionToken as sole write path (auth-entry-actions, app/auth) - canonical redirect to /auth on expiry/invalid token - booking-wizard POST wired to real API - provider-screen useEffect navigation fix - null/shape guards in booking/providers screens - trailing duplicate fragments cleaned up Refs: PR#3 must-fix comments
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
apps/product-app/src/features/booking/booking-wizard-actions.test.js (1)
41-47: Assert the serialized booking payload in the success test.This currently only locks down URL/method/auth. If
submitBooking()stops sending the composedrequestedServicepayload, this test still passes while the API contract is broken.🧪 Tighten the assertion
expect(fetchMock).toHaveBeenCalledWith( 'http://localhost:3000/api/v1/bookings', expect.objectContaining({ method: 'POST', - headers: expect.objectContaining({ authorization: 'Bearer tok-123' }), + headers: expect.objectContaining({ + 'content-type': 'application/json', + authorization: 'Bearer tok-123', + }), + body: JSON.stringify({ requestedService: 'Plumbing / Leaky faucet / today' }), }), );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/product-app/src/features/booking/booking-wizard-actions.test.js` around lines 41 - 47, The test currently only asserts URL/method/auth for submitBooking(); extend the assertion in booking-wizard-actions.test.js to verify the serialized booking payload is sent by checking the fetch call's body includes the composed requestedService data (e.g., expect.objectContaining({ body: JSON.stringify(expectedBookingPayload) }) or parse the body and match expectedBookingPayload). Locate the test that calls submitBooking() and add an assertion against fetchMock's arguments to ensure the request body includes the expected requestedService fields and full booking payload.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/product-app/src/features/auth/auth-entry-screen.js`:
- Around line 92-96: AuthEntryScreen currently renders a live "Create account"
CTA even when no handler is provided (onCreateAccount is undefined), causing a
silent no-op; update the component to guard the CTA by checking the
onCreateAccount prop (or make onCreateAccount required) and either hide or
disable the "Create account" button when onCreateAccount is falsy, and ensure
any internal handler (e.g., handleCreateAccount) early-returns if
onCreateAccount is not a function so clicks do nothing and cannot fail silently.
- Around line 96-100: The form enables sign-in based on email.trim() but
handleSignIn forwards the raw email, so trim the email before submitting: in the
handleSignIn function (which calls onSignIn) pass email: email.trim() (keep
password and role unchanged) so trailing/leading whitespace is removed before
calling onSignIn.
In `@apps/product-app/src/features/booking/booking-screen.js`:
- Around line 24-26: The early return when session.status !== 'authenticated'
makes the local error set on signOut() in BookingScreen unreachable so the
"session-expired" banner never renders; instead of storing the failure reason in
BookingScreen, propagate it through the auth/navigation flow: update signOut()
to include the failure reason in the redirect (e.g. as state/query param) or
write it to your shared auth state/context/store that BookingScreen (or the auth
route) reads, and remove reliance on local component state for this message;
apply the same fix for the other signOut-related branch referenced (the block
around lines 33-38) so the banner can render after navigation.
---
Nitpick comments:
In `@apps/product-app/src/features/booking/booking-wizard-actions.test.js`:
- Around line 41-47: The test currently only asserts URL/method/auth for
submitBooking(); extend the assertion in booking-wizard-actions.test.js to
verify the serialized booking payload is sent by checking the fetch call's body
includes the composed requestedService data (e.g., expect.objectContaining({
body: JSON.stringify(expectedBookingPayload) }) or parse the body and match
expectedBookingPayload). Locate the test that calls submitBooking() and add an
assertion against fetchMock's arguments to ensure the request body includes the
expected requestedService fields and full booking payload.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 281f086f-a2c9-4e5d-ab77-9775344fdb1e
📒 Files selected for processing (15)
.agent/plans/quickwerk-pr3-mustfix-week-plan-2026-03-26.md.agent/reports/execution-reports/quickwerk-pr3-batch1-execution-2026-03-26.md.agent/reports/execution-reports/quickwerk-pr3-batch2-execution-2026-03-26.md.agent/reports/execution-reports/quickwerk-pr3-batch3-execution-2026-03-26.md.agent/reports/execution-reports/quickwerk-pr3-kickoff-2026-03-26.mdapps/product-app/app/auth.jsapps/product-app/app/booking-wizard.jsapps/product-app/src/features/auth/auth-entry-actions.jsapps/product-app/src/features/auth/auth-entry-screen.jsapps/product-app/src/features/booking/booking-screen.jsapps/product-app/src/features/booking/booking-wizard-actions.jsapps/product-app/src/features/booking/booking-wizard-actions.test.jsapps/product-app/src/features/booking/booking-wizard-screen.jsapps/product-app/src/features/provider/provider-screen.jsapps/product-app/src/shared/session-provider.js
✅ Files skipped from review due to trivial changes (1)
- .agent/plans/quickwerk-pr3-mustfix-week-plan-2026-03-26.md
🚧 Files skipped from review as they are similar to previous changes (7)
- apps/product-app/app/booking-wizard.js
- apps/product-app/app/auth.js
- apps/product-app/src/features/booking/booking-wizard-actions.js
- apps/product-app/src/features/booking/booking-wizard-screen.js
- apps/product-app/src/features/provider/provider-screen.js
- apps/product-app/src/features/auth/auth-entry-actions.js
- apps/product-app/src/shared/session-provider.js
Comment on lines
+96
to
+100
| const isSignInDisabled = isSigningIn || !email.trim() || !password; | ||
|
|
||
| if (result.errorMessage) { | ||
| setActionStatusMessage(`Sign-in warning: ${result.errorMessage}`); | ||
| return; | ||
| } | ||
| const handleSignIn = () => { | ||
| if (onSignIn) { | ||
| onSignIn({ email, password, role }); |
Contributor
There was a problem hiding this comment.
Trim the email before forwarding it.
Line 96 validates email.trim(), but Line 100 submits the raw string. A trailing space keeps the CTA enabled and turns into an avoidable auth failure.
✂️ Proposed fix
- onSignIn({ email, password, role });
+ onSignIn({ email: email.trim(), password, role });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/product-app/src/features/auth/auth-entry-screen.js` around lines 96 -
100, The form enables sign-in based on email.trim() but handleSignIn forwards
the raw email, so trim the email before submitting: in the handleSignIn function
(which calls onSignIn) pass email: email.trim() (keep password and role
unchanged) so trailing/leading whitespace is removed before calling onSignIn.
Comment on lines
24
to
26
| if (session.status !== 'authenticated') { | ||
| router.replace('/sign-in'); | ||
| return null; | ||
| } |
Contributor
There was a problem hiding this comment.
The session-expired message is unreachable on this path.
At Line 35 you set a local error, but signOut() immediately flips the session and Lines 24-26 return null while the route is being replaced. The banner at Lines 148-152 never renders, so users still land on /auth with no explanation. Carry the failure reason through navigation or shared auth state instead of storing it in BookingScreen.
Also applies to: 33-38
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/product-app/src/features/booking/booking-screen.js` around lines 24 -
26, The early return when session.status !== 'authenticated' makes the local
error set on signOut() in BookingScreen unreachable so the "session-expired"
banner never renders; instead of storing the failure reason in BookingScreen,
propagate it through the auth/navigation flow: update signOut() to include the
failure reason in the redirect (e.g. as state/query param) or write it to your
shared auth state/context/store that BookingScreen (or the auth route) reads,
and remove reliance on local component state for this message; apply the same
fix for the other signOut-related branch referenced (the block around lines
33-38) so the banner can render after navigation.
added 2 commits
March 28, 2026 15:40
- auth.js: add try/finally so loading resets on sign-in error - auth.js: wire handleCreateAccount to AuthEntryScreen - booking-wizard.js: wire onEdit for location picker - home-triage.js: add console.warn to onChangeAddress stub - booking-wizard-screen: category-aware STEPS (plumbing/electrical/default) - booking-wizard-screen: forward onEdit to LocationStep - packages/ui: export Shadow type; fix designTokens.color.surface -> colors.surface Refs: PR#3 must-fix comments (imKXNNY review)
- apps/admin-web: remove brittle .next/dev/types/routes.d.ts import; add .next/types/**/*.ts to tsconfig include - apps/product-app/app/active-job.js: guard numeric param parsing with toFiniteNumber helper to prevent NaN Refs: CodeRabbit review on PR #3
This was referenced Mar 28, 2026
This was referenced Apr 8, 2026
Merged
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
1 participant
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.
Summary
This PR lands the current QuickWerk UI sprint state end-to-end, including Stitch design references, new customer-facing screens, real API wiring for the new booking flow, and provider-flow runtime fixes.
What’s done
1) Stitch design intake (reference artifacts)
stitch-designs/screenshots/*andstitch-designs/html/*for 7 generated/collected screens.2) Product app UI rollout (Expo RN)
/auth/home-triage/provider-profile/booking-wizard/active-job3) Session architecture + auth wiring
src/shared/session-provider.js(React Context-based session state).app/_layout.js.auth-entry-actions.js.app/index.jsto route through context-backed/authflow.4) Booking flow real API wiring
booking-wizard-actions.jscallingPOST /api/v1/bookings./booking-wizardcompletion to create booking and route to/active-job.5) Runtime bug fixes from manual UX testing
/sign-in+ module-store mismatch.provider-screen(moved touseEffect),/auth.6) Dev workflow
apps/admin-webdev port remains on3001to avoid API port conflicts.Validation
pnpm --filter @quickwerk/product-app test✅ (47 tests)pnpm check✅ (workspace typecheck)expo export --platform web✅ (clean bundle export)Recommended next after merge
provider-partner-viewqueue UX (Stitch ref 05)apps/admin-web)Summary by CodeRabbit
New Features
Updates
Tests