168 lines
5.1 KiB
Markdown
168 lines
5.1 KiB
Markdown
# Modal Event Handling Audit
|
|
|
|
## Overview
|
|
|
|
This document provides a security and event-handling audit of all modals in the application to identify and prevent event propagation issues similar to the card-types bug.
|
|
|
|
## Audit Results
|
|
|
|
### ✅ FIXED: packages/app/src/pages/admin/card-types.vue
|
|
|
|
**Status**: FIXED in commit a85270e
|
|
|
|
**Issue**: Action buttons inside a list card were closing the modal immediately when clicked due to event propagation to parent modal-mask.
|
|
|
|
**Solution**: Added `.stop` modifier to all three action button tap handlers:
|
|
- Edit button: `@tap.stop="openEdit(ct)"`
|
|
- Toggle button: `@tap.stop="toggleActive(ct)"`
|
|
- Delete button: `@tap.stop="confirmDelete(ct)"`
|
|
|
|
**Root Cause Pattern**:
|
|
- List items contain action buttons
|
|
- Action buttons are inside list cards
|
|
- Modal-mask has `@tap.self="closeModal"`
|
|
- Event from action button bubbles up through list card to modal-mask
|
|
|
|
---
|
|
|
|
### ✅ SAFE: packages/app/src/pages/admin/week-template.vue
|
|
|
|
**Status**: NO ACTION NEEDED
|
|
|
|
**Structure**:
|
|
- Template list (lines 30-56) - separate from modal
|
|
- Modal (lines 65+) - below the list
|
|
- Event handlers on template action buttons cannot reach modal-mask
|
|
|
|
**Reasoning**: The action buttons for edit/delete/toggle are on items in the template list, which is spatially separated from the modal-mask. The events cannot propagate upward to reach the modal-mask since the modal is rendered separately below the list.
|
|
|
|
---
|
|
|
|
### ✅ SAFE: packages/app/src/pages/admin/members.vue
|
|
|
|
**Status**: NO ACTION NEEDED
|
|
|
|
**Structure**:
|
|
- Members list uses `@tap="openDetail(m)"` on entire row element
|
|
- Modal is triggered with delay to handle event properly
|
|
- List items are separate from modal-mask
|
|
|
|
**Reasoning**: The entire member row has a single tap handler. The modal is opened as a detail view, not as an overlay that interferes with list item events. The architecture prevents event propagation issues.
|
|
|
|
---
|
|
|
|
### ✅ SAFE: components/BookingConfirmPopup.vue
|
|
|
|
**Status**: NO ACTION NEEDED (Special-case popup component)
|
|
|
|
**Structure**: Dedicated popup component with internal button handlers
|
|
|
|
---
|
|
|
|
## Event Propagation Risk Pattern
|
|
|
|
🚨 **RISK PATTERN** - High risk of event propagation issues:
|
|
|
|
```vue
|
|
<!-- List of items with action buttons -->
|
|
<view class="item-list">
|
|
<view v-for="item in items" :key="item.id" class="item-card">
|
|
<view class="item-actions">
|
|
<view @tap="handleAction1(item)">Action 1</view>
|
|
<view @tap="handleAction2(item)">Action 2</view>
|
|
</view>
|
|
</view>
|
|
</view>
|
|
|
|
<!-- Modal that appears on top -->
|
|
<view v-if="showModal" class="modal-mask" @tap.self="closeModal">
|
|
<view class="modal">...</view>
|
|
</view>
|
|
```
|
|
|
|
When an action button is tapped, the event bubbles: action button → item card → item-list → modal-mask
|
|
|
|
**SOLUTION**: Add `.stop` modifier to prevent bubbling:
|
|
```vue
|
|
<view @tap.stop="handleAction1(item)">Action 1</view>
|
|
```
|
|
|
|
---
|
|
|
|
## Preventive Measures
|
|
|
|
### 1. Code Review Checklist
|
|
|
|
When implementing modals with action buttons in lists:
|
|
|
|
- [ ] List items contain action buttons/clickable elements
|
|
- [ ] Modal-mask has `@tap.self="closeModal"` handler
|
|
- [ ] Check if tap events can bubble from buttons → modal-mask
|
|
- [ ] Add `.stop` modifier if event propagation risk exists
|
|
|
|
### 2. Testing Strategy
|
|
|
|
For any modal with nearby action buttons:
|
|
|
|
```
|
|
Test Scenario:
|
|
1. Click/tap action button that opens modal
|
|
2. Verify modal opens and stays open
|
|
3. Verify you can interact with modal content
|
|
4. Verify clicking outside modal (on mask) closes it
|
|
5. Verify multiple rapid clicks on action buttons don't cause flicker
|
|
```
|
|
|
|
### 3. Best Practices
|
|
|
|
```vue
|
|
<!-- ✅ SAFE: Action button prevents event propagation -->
|
|
<view @tap.stop="openModal(item)">Edit</view>
|
|
|
|
<!-- ❌ RISKY: Event can bubble to modal-mask -->
|
|
<view @tap="openModal(item)">Edit</view>
|
|
|
|
<!-- ✅ ALTERNATIVE: Use .prevent for links/special handlers -->
|
|
<view @tap.prevent="handleSpecial">Special</view>
|
|
|
|
<!-- ✅ ALTERNATIVE: Defer modal opening to next tick -->
|
|
<script>
|
|
async function openModal(item) {
|
|
editTarget.value = item
|
|
await nextTick()
|
|
showModal.value = true
|
|
}
|
|
</script>
|
|
```
|
|
|
|
---
|
|
|
|
## Summary
|
|
|
|
| File | Issue | Status | Solution |
|
|
|------|-------|--------|----------|
|
|
| card-types.vue | Event propagation | ✅ FIXED | Added `.stop` to 3 buttons |
|
|
| week-template.vue | N/A - Separate structure | ✅ SAFE | No action needed |
|
|
| members.vue | N/A - Single tap handler | ✅ SAFE | No action needed |
|
|
|
|
**Total Affected**: 1 file
|
|
**Total Fixed**: 1 file
|
|
**Total Safe**: 2 files
|
|
|
|
---
|
|
|
|
## Future Enhancements
|
|
|
|
1. **Automated Testing**: Add E2E tests for modal interactions
|
|
2. **ESLint Rule**: Consider adding custom rule to warn about `@tap` handlers on buttons inside modals
|
|
3. **Documentation**: Add event handling guidelines to project style guide
|
|
4. **Component Library**: Create a reusable `<Modal>` component with proper event handling built-in
|
|
|
|
---
|
|
|
|
## References
|
|
|
|
- Vue Event Handling: https://vuejs.org/guide/essentials/event-handling.html
|
|
- Event Modifiers: https://vuejs.org/guide/essentials/event-handling.html#event-modifiers
|
|
- Bug Fix Commit: a85270e - fix(admin): prevent edit modal from closing immediately on tap
|