stevengharris/MarkupEditor

Trouble placing an image at the top of the editor

Opened this issue · 14 comments

I've pretty much got everything in the editor working the way that I need, but I've run into one odd problem. If I place an image before typing any text, the image doesn't appear. Curiously, if I go back into the photo picker, that image is checked – so some part of the system knows it's been dealt with.

I know this is an abstract question to answer without seeing any code, but do you have any ideas where I might start looking for trouble?

Definitely difficult to say without any code 😬. I verified that in the SwiftUIDemo, when the document is empty (i.e., its contents is <p><br></p>), the action of selecting a local image does result in the image being inserted into the document. Maybe a place to start debugging-by-comparison with your code is with MarkupWKWebView.showImagePopover():

    /// Show the default link popover using the ImageViewController.
    @objc public func showImagePopover() {
        MarkupEditor.showInsertPopover.type = .image    // Does nothing by default
        startModalInput()                               // Required to deal with focus properly for popovers
        let imageVC = ImageViewController()
        imageVC.modalPresentationStyle = .popover
        imageVC.preferredContentSize = CGSize(width: 300, height: 140 + 2.0 * MarkupEditor.toolbarStyle.buttonHeight())
        guard let popover = imageVC.popoverPresentationController else { return }
        popover.delegate = self
        popover.sourceView = self
        // The sourceRect needs a non-zero width/height, but when selection is collapsed, we get a zero width.
        // The selectionState.sourceRect makes sure selRect has non-zero width/height.
        popover.sourceRect = MarkupEditor.selectionState.sourceRect ?? bounds
        closestVC()?.present(imageVC, animated: true)
    }

This is the code that the MarkupDelegate invokes to bring up the popover next to the insert-image button. The startModalInput() is important, because it ensures that MU.currentSelection is set beforehand so that focus can be restored in the view after the non-view interaction (e.g., with the popover and the picker both) is completed. If you are using a photo picker instead of the document picker, you also need to be sure your flow to get to it does startModalInput() beforehand.

If this isn't the issue, then following the flow of the DemoContentView, I use .pick(isPresented: $selectImage.value, documentTypes: MarkupEditor.supportedImageTypes, onPicked: imageSelected(url:), onCancel: nil) to bring up the ImageViewController which presents the UIDocumentPicker when the Select... button is pressed in it. That picker executes DemoContentView.imageSelected(url:) with the url of the selected local file, which ultimately invokes MarkupWKWebView.insertLocalImage(url:handler:). That method copies from the url into a uniquely named image file in the cache directory and then tells the JavaScript side to insert the IMG element with src pointing at that file. Presumably the only difference in your flow is that you're using a photo picker to select the URL, but I would think debugging your flow perhaps side-by-side with one you can see does work might be the most direct path to uncover the issue.

The problem is only when I insert an image at the very top of a new document. I hadn't been sure what to put in a new, empty document and had been using a pair of P tags with a non-breaking space in between. Thanks for the clarification that it should be a BR.

When I put some text first and then an image, the HTML comes out simply includes a nice IMG tag for the image, like it's supposed to. When I put an image before anything else I get this curious tag:

<title>&lt;span class="resize-container" tabindex="-1"&gt;&lt;span class="resize-handle resize-handle-nw"&gt;&lt;/span&gt;&lt;span class="resize-handle resize-handle-ne"&gt;&lt;/span&gt;&lt;img src="947CEACB-CC11-4FBA-AA57-EBB1A880EC98.png" class="resize-image" tabindex="-1" width="2800" height="1400"&gt;&lt;span class="resize-handle resize-handle-sw"&gt;&lt;/span&gt;&lt;span class="resize-handle resize-handle-se"&gt;&lt;/span&gt;&lt;/span&gt;My first styled page</title>
<link rel="stylesheet" href="markup.css">
<p><br></p></BODY>
</HTML>

I'm using an image picker instead of the popover, so I'll look into your previous comment.

That content is badly wrong and indicates to me that the body is being modified, not the editor div. In any case, I would recommend using the MarkupWKWebView.emptyDocument method to get an empty document rather than have your code be aware of an internal implementation detail about how to create a selectable but empty element in the underlying DOM. This is what the FileToolbar does in the demo.

Okay, I've been looking at the sample project, but still can't figure it out, partly because I have a different interface into the Markup editor. I have a scrolling list of existing documents, when the user taps on one, I need to load that into the Markup Editor. I also have a New button, which I use to create a new document.

Here's how I call the editor:

struct PersLogWYSIWYGEditor : View {
   @ObservedObject var theCurrentLog = PersonalLogViewController.shared
   @ObservedObject var theCurrentSkin = SkinWrapper.shared
   @ObservedObject var thePreferences = PreferencesViewController.shared
   
   var body: some View {
      MarkupEditorView(markupDelegate: self, html: $theCurrentLog.logMarkupHTML, whichEditor: "Log", whichSkin: theCurrentSkin.currentSkin.skinName, selectAfterLoad: true, resourcesUrl: theCurrentLog.logMarkupResourcesURL, id: "Log")
         .padding(.leading, theCurrentSkin.currentSkin.logEditorTextFieldLeadingPadding)
         .padding(.trailing, theCurrentSkin.currentSkin.logEditorTextFieldTrailingPadding)
         .padding(.bottom, theCurrentSkin.currentSkin.logEditorBottomPadding[thePreferences.layoutChoice])
         .onDisappear { MarkupEditor.selectedWebView = nil }
      
   }
   private func setRawText(_ handler: (()->Void)? = nil) {
       MarkupEditor.selectedWebView?.getHtml { html in
          theCurrentLog.logMarkupRawText = attributedString(from: html ?? "")
           handler?()
       }
   }
   
   private func attributedString(from string: String) -> NSAttributedString {
       // Return a monospaced attributed string for the rawText that is expecting to be a good dark/light mode citizen
       var attributes = [NSAttributedString.Key: AnyObject]()
       attributes[.foregroundColor] = UIColor.label
       attributes[.font] = UIFont.monospacedSystemFont(ofSize: StyleContext.P.fontSize, weight: .regular)
       return NSAttributedString(string: string, attributes: attributes)
   }
}

Before that call I load up theCurrentLog.logMarkupHTML with the contents of the document they've selected from the picker or, if they've hit the New button, I put in


As you can see, Ive had to add some additional properties to MarkupEditor to get some layout and formatting stuff done. At no time am I calling MarkupEditor.selectedWebView?.emptyDocument() anywhere.

Does that make the problem any clearer?

Sorry to just come back with generic debugging advice on this, but this just mainly says to me that something is going on either in the parts loading theCurrentLog.logMarkupHTML or in the mods to MarkupEditor.

When I find myself this kind of situation (which with SwiftUI is more often than I want to admit), I am usually forced into starting from a working version in a completely new project, and progressively copy/pasting in the pieces in that make mine different. For example, I'd build a little view that uses the FileToolbar from the demo, and make sure that creating a new document followed by image insert works properly. Then I'd add a button for the photo picker and make that work properly. None of that would include any of the mods to MarkupEditorView you've made, just the vanilla MarkupEditor from main with code added to support the photo picker. Then I'd maybe find a way to fold-in your PersonalLogViewController.shared, so the html is set using it. Sooner or later you find what the issue is this way, and you can then go back to the original project and fix it there.

I was thinking about kicking this off myself, maybe just to get thru the use of the photo picker, to see if that uncovered anything, but I'm not sure that would help you, other than maybe give us a common set of code to look at for debugging. If that would help, I can do that and we can go from there.

Okay, I put together a demo of using the photo picker with the vanilla MarkupEditor and put it in this gist. If you use the MarkupEditor demo, then you can add all the files from the gist and put the PhotoPickerDemo in the SceneDelegate to open it up. The hacky PhotoPickerToolbar has an ugly new button that opens the photo picker and lets you choose a jpeg or png, copies the resulting image into the cacheUrl (now known as baseUrl), and then inserts the img element into the document that points to it. It works fine with an empty document.

I moved the minimum deployment target to 16.0 so I could use the SwiftUI PhotosPicker because it seems easiest. Anyway, maybe we can use this as a concrete example of code we can both see to discuss the problem as I was outlining above. Otherwise, I am always reduced to wild ass guesses about what's going on.

But wild ass guesses are the only way I ever get anything done!

Thanks for this, and thanks for the debugging advice, it's all very helpful. I'll go play around with this and see what I can find out. In the meantime, I've gotten embroiled in a problem in another part of the code (not related to MarkUp Editor) so it might be a couple of days. Thanks again!

I'm having some scope troubles with this code, but it might be because I'm not putting some things in the right place. I stuck PhotoPickerDelegate and PhotoPickerToolbar into my local MarkupEditor dependency. I put PhotoPickerDemo in my project and set up my SceneDelegate. I tacked the MarkupWKWebView extension (the one with insertPhotosItem) onto the end of the PhotoPickerDemo file but it's throwing but it's throwing an error saying it can't find baseURL in the let cachedImageURL statement. I tried moving that extension into the dependency, but then it couldn't find it.

Meanwhile, the .pick modifies in the PhotoPickerDemo are also throwing troubles.

Sorry if these are obvious things – I've not worked much with Packages before, so I'm a little confused about scope.

I used a cloned MarkupEditor repo and started from the .xcworkspace that includes the demos. I probably should have started with a clean repo and used the package. So, to make it easier, I created a new public repo for this that only has the package dependency. I removed the usage of .pick because it's an extension that was part of the demos that aren't part of the package. (As a result, the image insertion button in the toolbar doesn't work but is not relevant to us here, I think.)

This missing baseUrl is an indication that you are not using the current main or most up-to-date package. The Issue196 repo I created uses version 0.6.2. In doing this, I found the latest 0.7.0 has problems loading, I think because I built is using the optional strict concurrency checking. Anyway, 0.6.2 will also be fine for our purposes.

I'm trying to get sync'd and merged with your latest version, but have run into a little problem due to a customization I made to ToolbarImageButton. It looks like systemNameToButtonName(systemName: systemName!) is no longer working as a way to specify a button name. What can I use instead? This is the routine I was using before:

 Button(action: action, label: {
         if MarkupEditorView.whichSkin != "LCARS" {
            label()
               .frame(width: MarkupEditor.toolbarStyle.buttonHeight(), height: MarkupEditor.to[olbarStyle.buttonHeight())
         } else {
            ZStack {
               Capsule()
                  .foregroundColor(.red)
               HStack {
                  Spacer()
                  Text(systemNameToButtonName(systemName: systemName!))
                     .font(Font.custom("Futura-CondensedMedium", size: 18))
                     .padding(.top, 9)
                     .padding(.trailing, 13)
               }
            }
         }
      })](url)

systemNameToButtonName must be something you did to map system names like "photo" to something that makes more sense in your app. Maybe just a struct like...

struct MyButtonNames {
    private static let names: [String : String] = [
        "photo" : "Internal name for photo",
        "trash" : "Delete",
    ]

    static func internalName(for systemName: String) -> String {
        names[systemName] ?? ""
    }
}

Sorry, I hit send before I was ready on that last message.

I see that your insertPhotosItem function creates a cached image and then ultimately calls insertImage. I've got my own routine for creating a cached image, because I'm interfacing with a CoreData store and needing to make some little thumbnail files and things. After that I've been calling markupImageToAdd. I never do call insertImage. Could that be a problem? Again, I'm successfully inserting images, as long as there's some text in the document before the image button is pressed.

Well, insertImage is what actually modifies the DOM on the JavaScript side to contain the <img src="the cached filename"> in it. So if it ever shows up on your screen inside of the MarkupWKWebView, you must be doing that somewhere or calling insertImage to do it.

I guess the bottom line is: yes, you should be calling insertImage, because besides actually modifying the DOM by putting in the img tag, it does some other hocus pocus to make sure the insertion is undoable, the image can be selected/resized in the doc, and the does the addedImage callback only after the image is actually loaded using event listeners in _setSrc.