Fix mobile flashing bug and improve code efficiency #1
Conversation
johnzfitch
commented
Dec 14, 2025
johnzfitch
commented
- Add touch event handling to prevent dragover flickering on mobile
- Use dragenter/dragleave counter pattern to handle nested elements
- Add CSS touch-action and hover media queries for mobile
- Consolidate display state management with helper functions
- Use consistent .hidden class instead of inline display styles
- Add GPU acceleration hints for smoother transitions
- Add touch event handling to prevent dragover flickering on mobile - Use dragenter/dragleave counter pattern to handle nested elements - Add CSS touch-action and hover media queries for mobile - Consolidate display state management with helper functions - Use consistent .hidden class instead of inline display styles - Add GPU acceleration hints for smoother transitions
There was a problem hiding this comment.
Pull request overview
This PR addresses mobile interaction issues with the upload zone and refactors display state management for better consistency and maintainability. The changes prevent flickering and flashing on mobile devices through CSS media queries, touch event handling, and a drag counter pattern for nested elements.
Key Changes:
- Implement dragenter/dragleave counter pattern to prevent drag-and-drop flickering on nested elements
- Add mobile-optimized CSS with hover media queries, touch-action, and GPU acceleration hints
- Consolidate display state management with helper functions (showElement, hideElement, setVisible) and standardize on .hidden class usage
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| src/ui/styles.css | Optimize upload zone transitions, add mobile touch optimizations, wrap hover states in desktop-only media queries, add GPU acceleration hints |
| index.html | Add display helper functions, implement drag counter for flicker prevention, add touch event handlers for mobile, replace inline style.display with .hidden class throughout |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| elements.loadingOverlay.classList.remove('hidden'); | ||
|
|
There was a problem hiding this comment.
The loading overlay starts visible by default (no hidden class in HTML line 41), but this code removes the 'hidden' class on an element that doesn't have it. This line has no effect. The loading overlay should either start with the 'hidden' class in HTML, or this line should be removed as redundant.
| elements.loadingOverlay.classList.remove('hidden'); |
| setTimeout(() => { | ||
| elements.loadingOverlay.style.display = 'none'; | ||
| }, 300); | ||
| elements.loadingOverlay.classList.add('hidden'); |
There was a problem hiding this comment.
The loading overlay's CSS (.loading-overlay.hidden at line 808-811 of styles.css) uses opacity and pointer-events to hide, not display: none. This means adding the 'hidden' class will only fade it out but keep it in the layout. The previous implementation with manual opacity and display style manipulation was correct for this specific element. Either change the CSS to use display: none, or revert to the previous inline style approach for the loading overlay.
| elements.uploadZone.addEventListener('touchstart', (e) => { | ||
| // Allow normal touch for file selection | ||
| }, { passive: true }); |
There was a problem hiding this comment.
This touchstart event handler is empty with only a comment. If no action is needed, this event listener registration should be removed entirely. Empty handlers add unnecessary event listener overhead.
| elements.uploadZone.addEventListener('touchstart', (e) => { | |
| // Allow normal touch for file selection | |
| }, { passive: true }); |
| -webkit-tap-highlight-color: transparent; | ||
| touch-action: manipulation; | ||
| /* GPU acceleration for smoother transitions */ | ||
| will-change: border-color, background, box-shadow; |
There was a problem hiding this comment.
Using will-change for properties that are only triggered on hover/drag can negatively impact performance by keeping GPU layers active unnecessarily. The will-change property should be set dynamically when the transition is about to occur (e.g., on hover/dragenter) and removed after, rather than being permanently applied. Consider removing this or applying it conditionally via JavaScript.
| elements.uploadZone.addEventListener('dragleave', (e) => { | ||
| e.preventDefault(); | ||
| e.stopPropagation(); | ||
| dragCounter--; |
There was a problem hiding this comment.
The dragCounter can become negative if dragleave events fire without corresponding dragenter events (e.g., if the user drags from outside the browser window). This could cause the dragover class to never be removed when dragCounter becomes 0 again. Add a safeguard to prevent dragCounter from going below 0, such as: dragCounter = Math.max(0, dragCounter - 1);
| dragCounter--; | |
| dragCounter = Math.max(0, dragCounter - 1); |
