|
| 1 | +# Pull Request: Fix Copy-Code Console Error |
| 2 | + |
| 3 | +## 📝 Description |
| 4 | + |
| 5 | +This PR fixes the console error in the copy-code utility that was generating unnecessary noise when no code content was found to copy. The changes improve cross-browser compatibility and provide user-friendly visual feedback instead of developer-focused console errors. |
| 6 | + |
| 7 | +**Key Improvements:** |
| 8 | +- Replace console.error with user-friendly visual feedback |
| 9 | +- Add secure context check for modern clipboard API |
| 10 | +- Enhance fallback method for older browsers and HTTP contexts |
| 11 | +- Add mobile device support with setSelectionRange |
| 12 | +- Implement visual success/error states on copy buttons |
| 13 | +- Improve error handling with graceful degradation |
| 14 | + |
| 15 | +Fixes console noise when copy operation fails and ensures copy functionality works across all browsers including Firefox, Safari, and older browser versions. |
| 16 | + |
| 17 | +## 🔗 Related Issue |
| 18 | + |
| 19 | +Fixes #493 - Copy-Code Error Handling |
| 20 | + |
| 21 | +## 🔄 Type of Change |
| 22 | + |
| 23 | +- [x] 🐛 Bug Fix |
| 24 | +- [x] ⚡ Performance Improvement |
| 25 | +- [x] ♿ Accessibility Enhancement |
| 26 | + |
| 27 | +## 📷 Visual Changes |
| 28 | + |
| 29 | +<details> |
| 30 | +<summary>Screenshots / GIFs</summary> |
| 31 | + |
| 32 | +**Before:** Console error appears when copy fails |
| 33 | +**After:** Visual button feedback (green "Copied!" or red "Failed") with no console errors |
| 34 | + |
| 35 | +</details> |
| 36 | + |
| 37 | +## 🧪 Testing Performed |
| 38 | + |
| 39 | +### 📱 Browser Compatibility |
| 40 | + |
| 41 | +- [x] Chrome (Version: Latest) |
| 42 | +- [x] Firefox (Version: Latest) |
| 43 | +- [x] Safari (Version: Latest) |
| 44 | +- [x] Edge (Version: Latest) |
| 45 | +- [x] Mobile Chrome (Device: Android/iOS) |
| 46 | +- [x] Mobile Safari (Device: iOS) |
| 47 | + |
| 48 | +### 🖥️ Responsive Design |
| 49 | + |
| 50 | +- [x] Desktop (1200px+) |
| 51 | +- [x] Tablet (768px - 1199px) |
| 52 | +- [x] Mobile (320px - 767px) |
| 53 | + |
| 54 | +### ✅ Test Cases |
| 55 | + |
| 56 | +1. Copy code blocks in modern browsers (Chrome, Edge, Firefox) |
| 57 | +2. Copy code blocks in older browsers with clipboard API disabled |
| 58 | +3. Copy code blocks on mobile devices |
| 59 | +4. Test copy functionality over HTTP vs HTTPS |
| 60 | +5. Test with malformed code blocks (missing data-code attribute) |
| 61 | +6. Verify no console errors appear during failed copy operations |
| 62 | + |
| 63 | +## ♿ Accessibility |
| 64 | + |
| 65 | +- [x] Proper heading hierarchy maintained |
| 66 | +- [x] ARIA labels added where needed (aria-label on copy buttons) |
| 67 | +- [x] Color contrast requirements met |
| 68 | +- [x] Keyboard navigation works correctly |
| 69 | +- [x] Screen reader testing performed |
| 70 | + |
| 71 | +## 📋 PR Checklist |
| 72 | + |
| 73 | +- [x] My code follows the project's coding style guidelines |
| 74 | +- [x] I have tested these changes locally |
| 75 | +- [x] I have updated the documentation accordingly |
| 76 | +- [x] My changes generate no new warnings or console errors |
| 77 | +- [x] I have added tests that prove my fix/feature works |
| 78 | +- [x] All existing tests pass successfully |
| 79 | +- [x] I have checked for and resolved any merge conflicts |
| 80 | +- [x] I have optimized images/assets (if applicable) |
| 81 | +- [x] I have validated all links are working correctly |
| 82 | + |
| 83 | +## 💭 Additional Notes |
| 84 | + |
| 85 | +This fix addresses the user experience issue where developers would see console errors during normal website usage. The new implementation: |
| 86 | + |
| 87 | +1. **Gracefully handles all browser scenarios** without generating console noise |
| 88 | +2. **Provides immediate visual feedback** to users about copy success/failure |
| 89 | +3. **Maintains backward compatibility** with older browsers |
| 90 | +4. **Improves mobile experience** with enhanced text selection |
| 91 | + |
| 92 | +The changes are minimal and focused, ensuring no breaking changes to existing functionality while significantly improving the user experience across all browsers and devices. |
| 93 | + |
| 94 | +## 🔧 Technical Details |
| 95 | + |
| 96 | +### Before (Issues): |
| 97 | +```javascript |
| 98 | +if (!codeContent) { |
| 99 | + console.error('No code content found to copy'); // ❌ Console noise |
| 100 | + return; |
| 101 | +} |
| 102 | +``` |
| 103 | + |
| 104 | +### After (Fixed): |
| 105 | +```javascript |
| 106 | +if (!codeContent) { |
| 107 | + showErrorFeedback(button, 'No content to copy'); // ✅ User feedback |
| 108 | + return; |
| 109 | +} |
| 110 | +``` |
| 111 | + |
| 112 | +### Cross-Browser Compatibility: |
| 113 | +- **Modern browsers (HTTPS)**: Uses `navigator.clipboard.writeText()` |
| 114 | +- **Older browsers / HTTP**: Falls back to `document.execCommand('copy')` |
| 115 | +- **Mobile devices**: Enhanced with `setSelectionRange()` for better text selection |
| 116 | +- **Failed operations**: Visual feedback instead of console errors |
| 117 | + |
| 118 | +--- |
| 119 | + |
| 120 | +### 📚 Reviewer Resources |
| 121 | +- [Contributing Guide](https://github.com/sugarlabs/www-v2/blob/main/docs/CONTRIBUTING.md) |
| 122 | +- [Style Guide](https://github.com/sugarlabs/www-v2/blob/main/docs/dev_guide.md) |
| 123 | +- [Community Chat](https://matrix.to/#/#sugarlabs-web:matrix.org) |
| 124 | + |
| 125 | +Thank you for contributing to the Sugar Labs website! 🎉 |
0 commit comments