stevengharris/MarkupEditor

Two toolbars...but only once

Closed this issue · 11 comments

I'm returning to this problem I mentioned a week or so ago, that I'd stuck on the back burner...

When I invoke the MarkupEditor for the first time, it comes in with a toolbar at both the top and bottom of the view. When I trace through, I find that the init routine for MarkupToolbar is being called twice. What's weird is that if I dismiss the editor and re-invoke it, everything's fine, and it stays fine for as long as the app runs (this is on an iPad). Tracing it on these subsequent invokes shows that, sure enough, init is only being called once.

Any ideas where I might poke around next?

I wonder if toolbarPosition is being changed somewhere, like maybe it defaults to top and then you're setting it to bottom? I have a vague recollection of you setting the toolbar to the bottom. If so, maybe you could try it at the top and see if the problem goes away as a way to narrow it down. The weird part is that you end up with two toolbars, and in particular that MarkupToolbar.init is actually called twice. Are you using MarkupEditorView so that one of the two conditionals installs the MarkupToolbar here?

    public var body: some View {
        VStack(spacing: 0) {
            if MarkupEditor.toolbarLocation == .top {
                MarkupToolbar(markupDelegate: markupDelegate).makeManaged()
                Divider()
            }
            MarkupWKWebViewRepresentable(markupDelegate: markupDelegate, wkNavigationDelegate: wkNavigationDelegate, wkUIDelegate: wkUIDelegate, userScripts: userScripts, configuration: markupConfiguration, html: html, placeholder: placeholder, selectAfterLoad: selectAfterLoad, resourcesUrl: resourcesUrl, id: id)
            if MarkupEditor.toolbarLocation == .bottom {
                Divider()
                MarkupToolbar(markupDelegate: markupDelegate).makeManaged()
            }
        }
    }

I also didn't realize this was on an iPad, so maybe there is a way that the inputAccessoryView for the keyboard is being set up and that is what one of the toolbars is. This is done in MarkupWKWebView.initForEditing(), so making sure accessoryView is nil would be another thing to check (assuming it's not supposed to be on there).

Well, for better or for worse, it's gotten weirder.

I thought your idea of changing the toolbar position back to .top was very clever. So in the onAppear that's attached to my MarkupEditorView call I commented out MarkupEditor.toolbarLocation = .none and stuck in MarkupEditor.toolbarLocation = .top just below it. When I ran that I still had two toolbars, just as before, but now they didn't go away after the first summoning. But that was at least a change. I went back and swapped out the .top line for the .none line...and now the problem is fixed. I only ever have one toolbar. I've installed it on two different iPads, then erased it and installed it again, I've run it on a few simulators, I've cleaned the project and restarted Xcode and it just stays working.

The obvious response is "well you must have changed something else," but I really feel quite certain that I didn't. I'm so paranoid about making any changes to your code that I never do that lightly, and I don't see anything different in any of mine. I can't help but wonder if Xcode just had some symbol stuck somewhere that it wasn't updating. However, there's a complication...

As I've mentioned before, I have two places where I'm using MarkUpEditor and they both have had this same problem. (Curiously, they do seem to share a toolbar. The first launch of one of the editors cures the second editor of the dual toolbar problem.) While the first editor is fixed, the second is still broken. I was very excited by the idea of reproducible results, but when I tried the same changes there, it didn't work.

So now this is all starting to feel like sorcery and I'm worried I need to go sacrifice a goat, except that I'm a vegetarian.

In your code you have a comment that says that, if you're using multiple instances of the editor view that you should set MarkupEditorToolbar.toolbarLocation to .none and work with MarkupToolbarUIView directly. I'm pretty sure this is why I've had it set to none and I have vague recollections of setting some other property to .bottom, but I can't for the life of me find it. Any tips would be welcome. My next thought was to find that, set it back to its default and try setting the MarkupEditor.toolbarLocation to .top again.

You should set the toolbar location in your AppDelegate or SceneDelegate and I guess all the toolbar weirdness will go away. See, for example, AppDelegate.init() in the SharedDemo content as a good place to set MarkupEditor.toolbarLocation.

Okay, that's better. I still have two toolbars, but now they're both at the bottom.

I stuck MarkupEditor.toolbarLocation = .bottom into the init of my AppDelegate. Curiously, the two toolbars are now always there – the extra doesn't go away the next time I invoke the editor. Tracing it shows that the MarkupEditor init is still firing twice.

With that in place, I took out the MarkupEditor.toolbarLocation = .none call from the onAppear method in my MarkupEditorView call. That left me with no toolbars at all.

You definitely should not be setting toolbarLocation more than once, and that should be done as early as possible. So removing it from .onAppear is the right thing to do. If you want to manage it yourself (i.e., you have one toolbar for multiple MarkupEditorView), then you want to set it to .none and create the MarkupToolbar yourself when you create some SwiftUI view that contains your two MarkupEditorView (neither of which will have a toolbar).

Here's an example of two MarkupEditorViews and a single MarkupToolbar at the bottom. When you click in one of the views, it sets the selectedWebView so the toolbar operates on it. I used the init method to set the toolbarLocation to .none so I didn't have to deal with AppDelegate or SceneDelegate when showing you an example, but typically, you should set it in one of those and not in init. I ran this by modifying the example SwiftUIDemo SceneDelegate to say let contentView = MultiEditorView().

import SwiftUI
import MarkupEditor

struct MultiEditorView: View {
    var body: some View {
        MarkupEditorView(markupDelegate: self)
        MarkupEditorView(markupDelegate: self)
        MarkupToolbar()
    }
    
    init() {
        MarkupEditor.toolbarLocation = .none
    }
}

extension MultiEditorView: MarkupDelegate {
    
    func markupClicked(_ view: MarkupWKWebView) {
        MarkupEditor.selectedWebView = view
    }
    
}

Thank you, that's very helpful. What if I have two separate views MarkupEditorViews and I want them each to have their own toolbar? So I've got one sitting in a view on one part of the screen, and another sitting in a different view on another part of the screen. I've stripped the toolbar down to just a few things, so I'd like to have it in both places.

This screenshot might help all this make more sense. In that shot, the upper pane is showing a WKWebView. If you hit the edit button to the right, that WebView is replaced with your MarkupEditor. I show the toolbar at the bottom of it.

That Library section shows a list of articles. If you tap on one, that list is replaced with a WKWebView, which also has an edit button. Tap on that buttont, and the Library webview is replaced with a MarkupEditor, which has its own little toolbar. If trying to get toolbars in both places is a problem, I don't mind limiting editing to only one viewer at a time so that only one toolbar is ever visible, but the toolbar would still need to appear in the appropriate pane.

I don't see a screenshot, but it's more about how you do this with SwiftUI, I think. Here's a variation on what I posted before that lets you switch between a WebView (a ViewRepresentable of a WKWebView) and a MarkupEditorView. Each MarkupEditorView has a bottom toolbar that only deals with that MarkupEditorView.

import SwiftUI
import MarkupEditor
import WebKit

struct MultiEditorView: View {
    @State var editing1: Bool = false
    @State var editing2: Bool = false
    var body: some View {
        if editing1 {
            MarkupEditorView(markupDelegate: self)
                .overlay(alignment: .topTrailing) {
                    Button(action: { editing1.toggle() }) {
                        Label("Edit", systemImage: "square.and.pencil")
                    }
                }
        } else {
            WebView(html: "<h1>This is WebView 1</h1>")
                .overlay(alignment: .topTrailing) {
                    Button(action: { editing1.toggle() }) {
                        Label("Edit", systemImage: "square.and.pencil")
                    }
                }
        }
        if editing2 {
            MarkupEditorView(markupDelegate: self)
                .overlay(alignment: .topTrailing) {
                    Button(action: { editing2.toggle() }) {
                        Label("Edit", systemImage: "square.and.pencil")
                    }
                }
        } else {
            WebView(html: "<h1>This is WebView 2</h1>")
                .overlay(alignment: .topTrailing) {
                    Button(action: { editing2.toggle() }) {
                        Label("Edit", systemImage: "square.and.pencil")
                    }
                }
        }
    }
    
    init() {
        MarkupEditor.toolbarLocation = .bottom
    }
}

extension MultiEditorView: MarkupDelegate {
    
    func markupClicked(_ view: MarkupWKWebView) {
        MarkupEditor.selectedWebView = view
    }
    
}

struct WebView: UIViewRepresentable {
    let url: URL?
    let html: String?
    
    private let dateFormatter: DateFormatter = {
        let formatter = DateFormatter()
        formatter.timeStyle = .medium
        return formatter
    }()
    
    init(url: URL? = nil, html: String? = nil) {
        self.url = url
        self.html = html
    }
    
    func makeUIView(context: Context) -> WKWebView  {
        let wkWebView = WKWebView()
        if let url {
            let request = URLRequest(url: url)
            wkWebView.load(request)
        } else if let html {
            wkWebView.loadHTMLString(html, baseURL: nil)
        } else {
            wkWebView.loadHTMLString("<h1>WebView created at \(dateFormatter.string(from: Date()))</h1>", baseURL: nil)
        }
        return wkWebView
    }
    
    func updateUIView(_ uiView: WKWebView, context: Context) {
    }
}

The MarkupEditor is really by design set up for one MarkupToolbar, which is why the selectedWebView is a static on MarkupEditor. Perhaps this was a mistake, but it means that while you can display one for each of your MarkupEditorViews, there should be only one showing at a time. By definition, if two are showing, they will both reflect the selectionState for the single selectedWebView.

Here's a modified version of the MultiEditorView example that makes sure there is only one MarkupToolbar at a time by making sure only one of the MarkupEditorViews is active. I also stuck in the logic to save the content when they switch. If you are still having trouble with toolbars, it would be good to work with an example like this so we can talk about the code.

struct MultiEditorView: View {
    @State var editing1: Bool = false
    @State var editing2: Bool = false
    @State var content1: String = "<h1>This is WebView 1</h1>"
    @State var content2: String = "<h1>This is WebView 2</h1>"
    var body: some View {
        if editing1 {
            MarkupEditorView(markupDelegate: self, html: $content1, id: "Editor1")
                .overlay(alignment: .topTrailing) {
                    Button(
                        action: {
                            saveContent(to: $content1, then: { editing1.toggle() })
                        },
                        label: {
                            Label("Edit", systemImage: "square.and.pencil")
                        }
                    )
                }
        } else {
            WebView(html: content1)
                .overlay(alignment: .topTrailing) {
                    Button(action: { editing1.toggle() }) {
                        Label("Edit", systemImage: "square.and.pencil")
                    }
                }
        }
        if editing2 {
            MarkupEditorView(markupDelegate: self, html: $content2, id: "Editor2")
                .overlay(alignment: .topTrailing) {
                    Button(
                        action: {
                            saveContent(to: $content2, then: { editing2.toggle() })
                        },
                        label: {
                            Label("Edit", systemImage: "square.and.pencil")
                        }
                    )
                }
        } else {
            WebView(html: content2)
                .overlay(alignment: .topTrailing) {
                    Button(action: { editing2.toggle() }) {
                        Label("Edit", systemImage: "square.and.pencil")
                    }
                }
        }
    }
    
    init() {
        MarkupEditor.toolbarLocation = .bottom
    }
    
    func saveContent(to binding: Binding<String>, then handler: (()->Void)? = nil) {
        MarkupEditor.selectedWebView?.getHtml { content in
            binding.wrappedValue = content ?? ""
            handler?()
        }
    }
}

extension MultiEditorView: MarkupDelegate {
    
    func markupLostFocus(_ view: MarkupWKWebView) {
        if view.id == "Editor1" {
            saveContent(to: $content1, then: { editing1.toggle() })
        } else {
            saveContent(to: $content2, then: { editing2.toggle() })
        }
    }
    
}

Thanks, I appreciate the extra code. I've been following your first example and trying to get my app re-structured to make it fit that architecture, but it's taking a while to get everything re-arranged. I'm fine with only one editor being available at a time. Earlier today, I swapped out any of the bits of your code that I had modified with fresh copies, but I still had the same problem, so at least now I know it's not something that I've screwed up. I think your hunch that it's a SwiftUI thing seems right. I'll let you know once I get all this rebuilt. In the meantime, here's another try at the screenshot – don't know why it didn't take the link before.

http://karlthecloud.knowsitall.info:880/IMG_238B30DA6614-1.jpeg

Good news! It's fixed! And as you were probably suspecting, it was all my fault. However, I couldn't have figured it out without your last couple of examples.

Because I wasn't initializing the toolbars properly, early on I had stuck in a MarkupToolbar().makeManaged() command, because that was the only way I could get the toolbar to appear. It was buried in a couple of nested views, so it was easy for me to overlook. Re-architecting things to initialize the toolbar much earlier, and taking out those .onAppear initializations and now removing that MarkupToolbar line has left everything working. I still don't understand why it was only a problem on the first invoking, or why it only affected one of my editors, but at this point, I'm fine with the mystery.

It was really helpful to me to see the super-stripped down implementations that you posted above. While I've found your included sample apps to be very helpful, you might consider sticking this totally barebones thing in the package because it makes initialization very clear.

As you have probably guessed, this is my first big iOS project and I'd like to say that I've learned more about architecture and design decisions from your code than from all the books/videos/etc that I've watched. You've given me ideas about LOTS of refactoring I can do, so thanks for the very effective lessons, in addition to this incredibly useful library.