-
Notifications
You must be signed in to change notification settings - Fork 2
fix(gotjunk): Red delete button now deletes visible filtered item #85
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Conversation
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
Changed all navigation and map control buttons from white backgrounds to dark gray (bg-gray-800) to fix white-on-white visibility issue reported by user. Changes: - LeftSidebarNav: bg-white/90 → bg-gray-800/90 for all 4 nav buttons - PigeonMapView: bg-white → bg-gray-800 for zoom/center/legend controls - White icons now clearly visible on dark backgrounds - Provides contrast against light map tile backgrounds Fixes user feedback: "icons no longer display on the map view... need a lite color background and not white on white" 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Fixed typo bug in handleReclassify where `classification` was used instead of `newClassification` parameter, causing incorrect discount/bid values. Added console logging to diagnose duplicate image creation: - Log when handleClassify is called (new items) - Log when handleReclassify is called (re-classification) - Log state updates to track if items are added/updated multiple times - Log IndexedDB saves to track storage operations This will help troubleshoot the issue where changing discount/bid creates duplicate images. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Added missing body scroll lock to PigeonMapView to complete the fullscreen overlay contract. This prevents Safari from clipping floating controls when the map is open. Changes: - Import useEffect from React - Lock document.body.style.overflow = 'hidden' on mount - Restore original overflow on unmount Completes the z-index fix implementation where all fullscreen overlays (ItemReviewer, FullscreenGallery, FullscreenViewer, PigeonMapView) now lock body scroll. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Fixed low-contrast sidebar icons on varied map tile backgrounds by implementing context-aware adaptive styling. Problem: - bg-gray-800/90 icons invisible on dark map tiles (parks, water, buildings) - Inconsistent visibility across mixed urban areas (light streets, dark foliage) - PR #40 fixed white-on-white but created dark-on-dark issue Solution: Added getButtonStyle() helper with conditional map-aware styling: - Map view: Inactive icons use bright bg-indigo-600/85 with strong borders - Other views: Inactive icons retain subtle bg-gray-800/90 - Active state: Always bright blue (unchanged) Changes: - LeftSidebarNav.tsx: Added getButtonStyle() helper function - All 4 buttons now use helper instead of inline ternary logic - Preserved Z_LAYERS.sidebar (2200) from PR #40 contract - No hardcoded z-index values Result: ✅ Icons visible on light streets, dark parks, blue water, gray buildings ✅ No regression on Browse/MyItems/Cart tabs ✅ Active (blue) distinguishable from inactive (indigo) on map ✅ Build: 413.49 kB │ gzip: 130.10 kB (2.68s) Pattern learned: UI over dynamic backgrounds needs context-aware styling. WSP References: WSP 50 (HoloIndex), WSP 22 (ModLog), WSP 64 (z-contract), WSP 87 (no vibecoding) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Added explicit pointer events to PigeonMapView to ensure proper event handling and confirmed z-index layering is correct. Z-index contract (already in place): - Sidebar: Z_LAYERS.sidebar (2200) - highest, always visible - Map: Z_LAYERS.mapOverlay (1600) - below sidebar Changes: - PigeonMapView.tsx: Added pointerEvents: "auto" to map container - Added missing z-index contract files (constants/zLayers.ts, styles/zindex-map.md) The sidebar SHOULD float above the map. If not visible in deployed version, redeploy with latest code from main branch. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Added comprehensive entry documenting 7 PRs merged in today's session: - PR #62: Icon size adjustment (12px → 16px) - PR #63: Instructions modal on every load + sidebar positioning - PR #64: Instructions modal with actual swipe button components - PR #65: Fixed modal overlap with camera orb - PR #66: Z-index hierarchy fix (modals above all controls) - PR #67: Instructions modal only on Browse tab - PR #68: Proper modal centering Documented: - Problem/solution for each PR - Code changes with before/after examples - WSP compliance references (WSP 22, 50, 64, 87) - User feedback integration process - Build metrics and zero regression validation WSP 22 Compliance: ModLog kept current with all session changes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Root Cause: - handleClassify cleared pendingClassificationItem at END of async function - If user double-tapped or tap fell through ActionSheet, second call would pass guard check before first call completed, creating duplicate items Fix: - Added isProcessingClassification flag to prevent concurrent calls - Clear pendingClassificationItem IMMEDIATELY after guard check - Wrap processing in try/finally to ensure flag is always reset - Log processing state for debugging Impact: - Eliminates duplicate item creation when: * User double-taps classification button * Tap falls through ActionSheet backdrop to button beneath * User rapidly taps after modifying discount/bid settings Tested: Build passes (vite build ✓) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
## Problem
**User Report**: "Red delete button doesn't delete the current visible item"
**Root Cause** (Data Source Mismatch):
- **UI displays**: `filteredBrowseFeed[0]` (e.g., first FREE item when filter active)
- **Swipe gesture deletes**: `filteredBrowseFeed[0]` ✓ Correct
- **Button deletes**: `browseFeed[0]` ❌ Wrong array - could be ANY item!
### The Bug in Detail
When classification filter is active (e.g., showing only "FREE" items):
```
browseFeed = [BID_item, FREE_item, DISCOUNT_item] // Unfiltered
filteredBrowseFeed = [FREE_item] // Filtered view
UI shows: FREE_item (filteredBrowseFeed[0])
User taps red button
App deletes: BID_item (browseFeed[0]) ❌ WRONG!
```
**Result**: User sees water bottle (FREE), taps delete, but app deletes something else entirely!
### Code Evidence
**Line 538** - UI renders:
```tsx
<ItemReviewer
item={filteredBrowseFeed[0]} // ✓ Shows filtered item
```
**Line 539** - Swipe handler:
```tsx
onDecision={() => handleBrowseSwipe(filteredBrowseFeed[0], ...)} // ✓ Correct array
```
**Line 715** - Button handler (BEFORE):
```tsx
handleBrowseSwipe(browseFeed[0], ...) // ❌ Wrong array!
```
## Solution
**Unified data source** - Both swipe and button use `filteredBrowseFeed[0]`:
**Change 1**: [App.tsx:714-716](modules/foundups/gotjunk/frontend/App.tsx#L714-L716)
```diff
- if (activeTab === 'browse' && browseFeed.length > 0) {
- handleBrowseSwipe(browseFeed[0], action === 'keep' ? 'right' : 'left');
+ if (activeTab === 'browse' && filteredBrowseFeed.length > 0) {
+ console.log('[GotJunk] Button action:', action, 'on item:', filteredBrowseFeed[0].id);
+ handleBrowseSwipe(filteredBrowseFeed[0], action === 'keep' ? 'right' : 'left');
```
**Change 2**: [App.tsx:725](modules/foundups/gotjunk/frontend/App.tsx#L725)
```diff
- hasReviewItems={activeTab === 'browse' ? browseFeed.length > 0 : myDrafts.length > 0}
+ hasReviewItems={activeTab === 'browse' ? filteredBrowseFeed.length > 0 : myDrafts.length > 0}
```
**Added instrumentation**: Console log shows which item is being deleted for debugging
## Test Results
**Build**: ✓ 418.33 kB (gzipped: 131.17 kB)
**Test Scenarios**:
- [ ] No filter active → Red button deletes first item in feed
- [ ] Filter = "FREE" → Red button deletes visible FREE item (not random item)
- [ ] Filter = "BID" → Red button deletes visible BID item
- [ ] Filter active but no results → Red/green buttons disabled
- [ ] Swipe left and red button produce identical behavior
## Verification
**Console output** will show:
```
[GotJunk] Button action: delete on item: abc123-def456
```
This confirms the button is targeting the same item the user sees.
## WSP Compliance
- ✓ WSP_00: Zen State (surgical fix, no vibecoding)
- ✓ WSP_50: Pre-action verification (HoloIndex + targeted grep)
- ✓ First Principles: Occam's Razor (single source of truth)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Fixes critical bug where red delete button deleted wrong item when classification filter active.
Before:
After:
Root Cause
Data source mismatch - Three different data sources:
filteredBrowseFeed[0]filteredBrowseFeed[0]browseFeed[0]When filter active:
Changes
App.tsx:714-716 - Fix button handler:
App.tsx:725 - Fix button disabled state:
Test Plan
[GotJunk] Button action: delete on item: abc123Build Output
WSP Compliance
🤖 Generated with Claude Code