feat: misc frontend UI fixes and improvements#56
Closed
cole-hackman wants to merge 2 commits intodevfrom
Closed
Conversation
- Fix duplicate sign-in messages on profile tab - Redirect unauthenticated users to profile on Create Listing (web) - Fix browser tab title persisting after leaving a listing - Style placeholder text with lighter grey across profile and create listing forms - Standardize Create Listing button to brand green (#154734) - Capitalize category and condition values on listing detail page - Add profile setup check before creating listings with visible warning banner - Replace Alert.alert with cross-platform showAlert for web compatibility - Add Delete button to My Listings with confirmation dialog
Contributor
📝 WalkthroughWalkthroughPlatform-specific unauthenticated create flow (web → /settings, native → alert), added listing deletion with confirmation and loading state, unified cross-platform showAlert utility, profile-gated listing creation with banner and pre-submit checks, settings placeholder/style tweaks, and web document.title management for listings. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as Client UI
participant ProfileQ as Profile Query (Convex)
participant API as Listings Mutation (Convex)
participant Alert as showAlert / Native Alert
User->>UI: Click "Create Listing"
UI->>ProfileQ: fetch currentProfile
ProfileQ-->>UI: currentProfile (undefined/null/object)
alt profile undefined
UI->>Alert: "Please wait — loading profile"
else profile null
UI->>Alert: "Complete profile"
UI-->>User: show profile banner / navigate to Profile
else profile present
UI->>API: createListing(payload)
API-->>UI: success / error
alt success
UI->>Alert: "Listing created"
else error
UI->>Alert: show error message (possibly PROFILE_SETUP_ERROR)
end
end
sequenceDiagram
participant User
participant UI as MyListings UI
participant Confirm as confirm() / Alert
participant API as deleteListing Mutation (Convex)
participant Alert as showAlert
User->>UI: Click "Delete" on a listing
UI->>Confirm: confirm dialog (web) / Alert.alert (native)
alt confirmed
UI->>API: deleteListing(listingId)
API-->>UI: success / error
alt success
UI-->>User: remove item / update state
else error
UI->>Alert: show error message
end
else cancelled
UI-->>User: no-op
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 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: 1
🧹 Nitpick comments (2)
frontend/app/(tabs)/settings.tsx (1)
64-80: Keep the signed-out reset aligned with the new empty year default.This branch still hard-codes
'2026', which fights the placeholder-only flow you introduced above. Resetting to''here too keeps every entry path consistent.Suggested cleanup
setLoadedProfileKey(null); setName(''); setEmail(''); setBio(''); setMajor(''); - setYear('2026'); + setYear(''); }, [isAuthenticated]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/`(tabs)/settings.tsx around lines 64 - 80, The signed-out reset in the useEffect should use the new empty-year default instead of the hard-coded '2026': update the branch that runs when !isAuthenticated to call setYear('') (alongside setLoadedProfileKey(null), setName(''), setEmail(''), setBio(''), setMajor('')) so the reset behavior matches the placeholder-only flow introduced earlier.frontend/app/listings/new.tsx (1)
27-33: Consider promotingshowAlertinto a shared dialog helper.
frontend/app/(tabs)/my-listings.tsxnow has the same web/native branching for browser dialogs. Pulling this into a small shared util would keep cross-platform messaging consistent and avoid a third copy later.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/listings/new.tsx` around lines 27 - 33, Extract the duplicated showAlert implementation into a small shared utility (e.g., export a showAlert function from a new module) and update both usages to import that single helper; specifically, move the logic in the showAlert function (the Platform.OS === 'web' window.alert branch and the Alert.alert branch) into the new shared function, ensure the new module imports Platform and Alert (and references window safely), export showAlert, and then replace the local showAlert implementations in component files (including the one currently named showAlert) with imports from the shared util so behavior remains identical across web/native.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/app/`(tabs)/my-listings.tsx:
- Around line 42-74: The UI allows starting multiple deletes concurrently
because only deletingId tracks a single row; fix by preventing new deletes while
a delete mutation is in flight: in handleDelete (and the delete button disabled
logic) short-circuit and return immediately if deleteListing.isLoading or a new
boolean isDeleting flag is true, and set that flag (or rely on
deleteListing.isLoading) at the start of handleDelete and clear it in finally;
update any delete-button disabled/ spinner conditions to use
(deleteListing.isLoading || deletingId !== null) so all delete buttons are
disabled while a delete is pending and the spinner stays consistent with
deletingId.
---
Nitpick comments:
In `@frontend/app/`(tabs)/settings.tsx:
- Around line 64-80: The signed-out reset in the useEffect should use the new
empty-year default instead of the hard-coded '2026': update the branch that runs
when !isAuthenticated to call setYear('') (alongside setLoadedProfileKey(null),
setName(''), setEmail(''), setBio(''), setMajor('')) so the reset behavior
matches the placeholder-only flow introduced earlier.
In `@frontend/app/listings/new.tsx`:
- Around line 27-33: Extract the duplicated showAlert implementation into a
small shared utility (e.g., export a showAlert function from a new module) and
update both usages to import that single helper; specifically, move the logic in
the showAlert function (the Platform.OS === 'web' window.alert branch and the
Alert.alert branch) into the new shared function, ensure the new module imports
Platform and Alert (and references window safely), export showAlert, and then
replace the local showAlert implementations in component files (including the
one currently named showAlert) with imports from the shared util so behavior
remains identical across web/native.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 62842e70-82e4-4908-965e-4917ea5e36f8
📒 Files selected for processing (5)
frontend/app/(tabs)/index.tsxfrontend/app/(tabs)/my-listings.tsxfrontend/app/(tabs)/settings.tsxfrontend/app/listings/[id].tsxfrontend/app/listings/new.tsx
- Extract showAlert into shared utils/showAlert.ts module - Prevent concurrent deletes in my-listings (guard + disable all buttons) - Fix year reset to empty string in settings signed-out useEffect - Remove unused Alert/Platform imports from new.tsx
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
frontend/app/(tabs)/my-listings.tsx (1)
68-69: Consider extracting the actual error message for better user feedback.The catch block discards the error details and shows a generic message. The backend's
verifyOwnershipcan return specific errors like "Listing not found" or "You are not the owner" which would be more informative to users.💡 Proposed improvement
- } catch { - showAlert('Error', 'Failed to delete listing. Please try again.'); + } catch (error) { + const message = error instanceof Error ? error.message : 'Failed to delete listing. Please try again.'; + showAlert('Error', message);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/`(tabs)/my-listings.tsx around lines 68 - 69, The catch block in the delete flow currently swallows the error and shows a generic alert; update the delete handler (the function that calls verifyOwnership / deleteListing and calls showAlert) to catch the error as a variable (e.g., catch (err)) and extract a useful message (err.message || err?.data?.message || String(err)) to pass into showAlert so users see backend messages like "Listing not found" or "You are not the owner", while keeping a fallback generic message.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@frontend/app/`(tabs)/my-listings.tsx:
- Around line 68-69: The catch block in the delete flow currently swallows the
error and shows a generic alert; update the delete handler (the function that
calls verifyOwnership / deleteListing and calls showAlert) to catch the error as
a variable (e.g., catch (err)) and extract a useful message (err.message ||
err?.data?.message || String(err)) to pass into showAlert so users see backend
messages like "Listing not found" or "You are not the owner", while keeping a
fallback generic message.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b8d05991-7ba8-4cf1-a602-a46373649259
📒 Files selected for processing (4)
frontend/app/(tabs)/my-listings.tsxfrontend/app/(tabs)/settings.tsxfrontend/app/listings/new.tsxfrontend/utils/showAlert.ts
Collaborator
|
Thanks cole! I merged your branch into my branch and fixed conflicts, since I was doing a UI overhaul. Closing this one as your changes will get merged with mine! |
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
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Linked Issues
No linked Linear issue — miscellaneous UI polish.
Summary
A collection of frontend UI fixes and quality-of-life improvements for the PolyBuys marketplace.
Profile tab (settings.tsx):
TextInputplaceholder text with lighter grey (#9aaa9f)Home feed (index.tsx):
#154734)Listing detail (
[id].tsx):Create listing (new.tsx):
window.alert()on web sinceAlert.alertis invisible in Expo webAlert.alertcalls with showAlert so validation messages actually appear on webMy Listings (my-listings.tsx):
window.confirm()on web, native Alert with Cancel/Delete on mobileSummary by CodeRabbit