Skip to content

Improve drag & drop image inlining functionality#244

Merged
schuyler merged 4 commits intomainfrom
claude/review-pr-240-fixes-R8s3i
Jan 1, 2026
Merged

Improve drag & drop image inlining functionality#244
schuyler merged 4 commits intomainfrom
claude/review-pr-240-fixes-R8s3i

Conversation

@schuyler
Copy link
Copy Markdown
Owner

Summary

This PR improves the drag & drop image inlining feature in the editor:

  • Multi-format support: JPEG, PNG, GIF, and WebP images are now supported with correct MIME types
  • Multiple file drops: All dropped image files are processed and inserted, not just the first one
  • Undo support: Uses insertText:replacementRange: for proper undo/redo functionality
  • Modern APIs: Updated from deprecated NSFilenamesPboardType/base64Encoding to NSPasteboardTypeFileURL/base64EncodedStringWithOptions:
  • Proper filtering: draggingEntered: now correctly filters for supported image types
  • Fallback behavior: Non-image files delegate to default NSTextView handling

Related Issue

Related to #243

Changes

File: MacDown/Code/View/MPEditorView.m

Change Description
Import Added <CoreServices/CoreServices.h> for UTI functions
awakeFromNib Register for NSPasteboardTypeFileURL instead of undefined NSDragPboard
draggingEntered: Check for supported image UTIs (JPEG, PNG, GIF, WebP)
performDragOperation: Process all dropped images with correct MIME types and undo support
imageUTIForFilePath: New helper to detect supported image UTIs from file path
mimeTypeForUTI: New helper to map UTI to MIME type

Manual Testing Plan

Basic Functionality

  1. Single image drop (JPEG, PNG, GIF, WebP): Should insert ![](data:image/<type>;base64,...)
  2. Multiple images: All images inserted on separate lines
  3. Non-image files: Should fall back to default NSTextView behavior
  4. Mixed files: Only images are inlined, others handled by default behavior

Undo/Redo

  1. Drop an image, press ⌘Z - should completely undo the insertion
  2. Drop multiple images, press ⌘Z - should undo all in one operation
  3. ⌘⇧Z should redo correctly

Edge Cases

  • Files with incorrect extensions (PNG named .jpg)
  • Empty files
  • Large files (5MB+)
  • Special characters in filenames

Review Notes

  • No documentation updates needed (per Harpo review)
  • Code review issues fixed: proper nil check for file data, fallback to super in draggingEntered:

- Support multiple image formats: JPEG, PNG, GIF, WebP
- Detect actual file type and use correct MIME type in data URLs
- Process all dropped image files, not just the first one
- Use insertText:replacementRange: for proper undo support
- Update to modern APIs (NSPasteboardTypeFileURL, base64EncodedStringWithOptions:)
- Add proper filtering in draggingEntered: to only accept supported images
- Fall back to default NSTextView behavior for non-image files

Related to #243
- Fix error handling: check if fileData is nil instead of checking error
  parameter (dataWithContentsOfFile:options:error: may not always set error)
- Fix fallback behavior in draggingEntered: call super instead of returning
  NSDragOperationNone to allow default NSTextView handling for non-images

Related to #243
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Dec 31, 2025

Code Coverage Report

Current Coverage: 51.47%

Coverage Details (Summary)
Name                                                                                                                                   Coverage            
-------------------------------------------------------------------------------------------------------------------------------------- ------------------- 
MASPreferences.bundle                                                                                                                  0.00% (0/0)         
MacDown 3000.app                                                                                                                       57.87% (7243/12516) 
    /Users/runner/work/macdown3000/macdown3000/MacDown/Code/Extension/NSColor+HTML.m                                                   94.05% (332/353)    
        +[NSColor(HTML) colorWithHexString:]                                                                                           94.12% (16/17)      
        +[NSColor(HTML) colorWithHTMLName:]                                                                                            89.13% (164/184)    
        __35+[NSColor(HTML) colorWithHTMLName:]_block_invoke                                                                           100.00% (152/152)   
    /Users/runner/work/macdown3000/macdown3000/Dependency/peg-markdown-highlight/HGMarkdownHighlighter.m                               82.47% (461/559)    
        styleparsing_error_callback                                                                                                    90.00% (9/10)       
        -[HGMarkdownHighlighter init]                                                                                                  92.86% (13/14)      
        -[HGMarkdownHighlighter initWithTextView:]                                                                                     83.33% (5/6)        
        -[HGMarkdownHighlighter initWithTextView:waitInterval:]                                                                        83.33% (5/6)        
        -[HGMarkdownHighlighter initWithTextView:waitInterval:styles:]                                                                 83.33% (5/6)        
        -[HGMarkdownHighlighter parseText:]                                                                                            100.00% (9/9)       
        -[HGMarkdownHighlighter convertOffsets:text:]                                                                                  26.19% (11/42)      
        -[HGMarkdownHighlighter requestParsing]                                                                                        100.00% (15/15)     
        __39-[HGMarkdownHighlighter requestParsing]_block_invoke                                                                       100.00% (12/12)     
        __39-[HGMarkdownHighlighter requestParsing]_block_invoke.11                                                                    100.00% (3/3)       
        __39-[HGMarkdownHighlighter requestParsing]_block_invoke.13                                                                    100.00% (4/4)       
        -[HGMarkdownHighlighter getClearFontTraitMask:]                                                                                88.89% (16/18)      
        -[HGMarkdownHighlighter clearHighlightingForRange:]                                                                            100.00% (31/31)     
        __51-[HGMarkdownHighlighter clearHighlightingForRange:]_block_invoke                                                           100.00% (8/8)       
        -[HGMarkdownHighlighter readClearTextStylesFromTextView]                                                                       100.00% (17/17)     
        -[HGMarkdownHighlighter applyHighlighting:withRange:]                                                                          80.00% (60/75)      
        __53-[HGMarkdownHighlighter applyHighlighting:withRange:]_block_invoke                                                         92.31% (12/13)      
        -[HGMarkdownHighlighter applyVisibleRangeHighlighting]                                                                         100.00% (16/16)     
        -[HGMarkdownHighlighter clearHighlighting]                                                                                     100.00% (3/3)       
        -[HGMarkdownHighlighter cacheElementList:]                                                                                     100.00% (6/6)       
        -[HGMarkdownHighlighter clearElementsCache]                                                                                    100.00% (2/2)       
        -[HGMarkdownHighlighter textViewTextDidChange:]                                                                                0.00% (0/11)        
        __47-[HGMarkdownHighlighter textViewTextDidChange:]_block_invoke                                                               0.00% (0/3)         
        -[HGMarkdownHighlighter textViewDidScroll:]                                                                                    27.27% (3/11)       
        __43-[HGMarkdownHighlighter textViewDidScroll:]_block_invoke                                                                   0.00% (0/6)         
        __43-[HGMarkdownHighlighter textViewDidScroll:]_block_invoke_2                                                                 0.00% (0/3)         
        -[HGMarkdownHighlighter getDefaultStyles]                                                                                      100.00% (27/27)     
        -[HGMarkdownHighlighter applyStyleDependenciesToTargetTextView]                                                                92.86% (13/14)      
        -[HGMarkdownHighlighter setStyles:]                                                                                            100.00% (8/8)       
        -[HGMarkdownHighlighter getDefaultSelectedTextAttributes]                                                                      100.00% (7/7)       
        -[HGMarkdownHighlighter handleStyleParsingError:]                                                                              100.00% (12/12)     
        -[HGMarkdownHighlighter applyStylesFromStylesheet:withErrorHandler:]                                                           86.05% (74/86)      
        -[HGMarkdownHighlighter setTargetTextView:]                                                                                    100.00% (7/7)       
        -[HGMarkdownHighlighter parseAndHighlightNow]                                                                                  100.00% (3/3)       
        -[HGMarkdownHighlighter highlightNow]                                                                                          100.00% (3/3)       
        -[HGMarkdownHighlighter activate]                                                                                              100.00% (24/24)     
        -[HGMarkdownHighlighter deactivate]                                                                                            100.00% (18/18)     
    /Users/runner/work/macdown3000/macdown3000/Dependency/peg-markdown-highlight/HGMarkdownHighlightingStyle.m                         87.69% (57/65)      
        +[HGMarkdownHighlightingStyle colorFromARGBColor:]                                                                             100.00% (6/6)       
        -[HGMarkdownHighlightingStyle initWithType:attributesToAdd:toRemove:fontTraitsToAdd:]                                          88.89% (8/9)        
        -[HGMarkdownHighlightingStyle initWithStyleAttributes:baseFont:]                                                               86.00% (43/50)      

... (2382 more lines truncated)

📊 **Full coverage report available in workflow artifacts**

claude added 2 commits January 1, 2026 04:35
Per feedback from @wltb: regular file drags should insert the file path
(default NSTextView behavior), while Option+drag (copy operation) should
inline the image as a data URL.

This matches macOS conventions where Option+drag = copy.

Related to #243
Tests for MPEditorView's private helper methods:
- imageUTIForFilePath: tests for PNG, JPEG, GIF, WebP extensions
- mimeTypeForUTI: tests for UTI to MIME type mapping
- Round-trip tests (file path -> UTI -> MIME type)
- Edge cases: uppercase extensions, unsupported types, nil/empty inputs

Uses Testing category pattern to expose private methods for testing.

Related to #243
@schuyler schuyler merged commit 487a103 into main Jan 1, 2026
7 checks passed
@schuyler schuyler deleted the claude/review-pr-240-fixes-R8s3i branch January 1, 2026 04:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants