Conversation
There was a problem hiding this comment.
Pull request overview
This pull request updates the event handling for the material selection modal overlay to use onPointerDown instead of onClick. The change affects how users dismiss the modal - it will now close immediately when the pointer is pressed down rather than waiting for a complete click action.
Changes:
- Updated material-modal-overlay to use
onPointerDownevent handler instead ofonClick - Updated material-modal (inner container) to use
onPointerDownwithstopPropagation()instead ofonClick
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <div className="material-modal-overlay" onPointerDown={() => setDropdownOpen(false)}> | ||
| <div className="material-modal" onPointerDown={(e) => e.stopPropagation()} ref={menuRef} onScroll={(e) => { dropdownScrollRef.current = e.currentTarget.scrollTop }}> |
There was a problem hiding this comment.
Using onPointerDown instead of onClick for modal overlay dismissal changes the user experience - the modal now closes on pointer-down rather than on a complete click (pointer-down + pointer-up). This means users cannot change their mind by moving the pointer away before releasing. This is a deviation from common modal interaction patterns and may negatively impact user experience. Additionally, the settings modal at lines 605-606 still uses onClick for the same pattern, creating an inconsistency across the application. Consider whether this change is intentional and necessary, and if so, apply it consistently to both modals.
| <div className="material-modal-overlay" onPointerDown={() => setDropdownOpen(false)}> | |
| <div className="material-modal" onPointerDown={(e) => e.stopPropagation()} ref={menuRef} onScroll={(e) => { dropdownScrollRef.current = e.currentTarget.scrollTop }}> | |
| <div className="material-modal-overlay" onClick={() => setDropdownOpen(false)}> | |
| <div className="material-modal" onClick={(e) => e.stopPropagation()} ref={menuRef} onScroll={(e) => { dropdownScrollRef.current = e.currentTarget.scrollTop }}> |
No description provided.