Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Long press and Double Tap on space that is not on the same line as the text carat #106

Closed
JohnKuan opened this issue Apr 12, 2023 · 14 comments · Fixed by #124
Closed

Long press and Double Tap on space that is not on the same line as the text carat #106

JohnKuan opened this issue Apr 12, 2023 · 14 comments · Fixed by #124

Comments

@JohnKuan
Copy link
Contributor

Simulator Screen Shot - iPhone 14 Pro - 2023-04-12 at 18 03 17

Hi @stevengharris,

I am looking at how to modify the code to have the MarkupWKWebview respond on the space where it is empty. Currently on your main branch, it only registers the gestures and show the UIMenuController when it is performed on the same horizontal line as the text carat.

For context, I am taking reference from how iOS Notes App is behaving.

@JohnKuan
Copy link
Contributor Author

What I did was to add a long press tap gesture on MarkupWKWebView, then showMenu from UIMenuController

But it just does not have that same effect as the Notes app.

@JohnKuan
Copy link
Contributor Author

I also noticed that the Select | Select All options are not available. I think those are actions that are quite frequently used in a text editor.

@stevengharris
Copy link
Owner

Hi John. Regarding Select and SelectAll, I don't see Select showing up in Edit menus (e.g., Notes, Mail, Safari, Xcode), but I definitely forgot SelectAll. This can be enabled in MarkupWKWebView.canPerformAction(_:withSender:) by adding another case to the switch:

case #selector(UIResponderStandardEditActions.selectAll(_:)):
    return true

Unfortunately, this exposes an issue on iOS, which is that touching inside of the selected text only brings up the context menu rather than resets the selection. I'll check into this when I look at the other issues you brought up.

@JohnKuan
Copy link
Contributor Author

Hi Steve, for the Select to be show, its usually when we initiate the menu when the carat is on the edge of a word/character.

RPReplay_Final1681351613.MP4

I have tried to add the case in the switch logic in canPerformAction(_:withSender:) and it did showed up for select and select all options.

case #selector(UIResponderStandardEditActions.select(_:)), #selector(UIResponderStandardEditActions.selectAll(_:)):
                return super.canPerformAction(action, withSender: sender)

I can create a PR for this, if the logic is fine for you. I think resolving to the default super.canPerformAction method will address that issue of resetting the selection you are referring to.

@JohnKuan
Copy link
Contributor Author

However on to the main topic for this issue thread, I am looking to have empty spaces respond and display the menu, not just the longitudinal space where the carat is at.

@stevengharris
Copy link
Owner

Thanks for offering to do the PR on the select and selectAll. I still don't see it resetting the selection properly when touching inside of a selected area of text after the menu is dismissed, so I might look into that some more.

I will be looking at the menu display in the empty space. I will probably see if it behaves the same in a vanilla contenteditable WKWebView first just to make sure it's something I'm doing differently.

@JohnKuan
Copy link
Contributor Author

Thank you for looking into this.

@stevengharris
Copy link
Owner

Just to check, when I have something in the paste buffer and I do a long-press in MarkupEditorView/MarkupWKWebView below, I get a context menu at the top left that allows me to paste. It behaves similarly to the TextEditor below it. However, as soon as the caret shows up and the keyboard shows (i.e., the MarkupWKWebView actually has focus), then long-press only works on the line with the caret, which is not the same behavior as the TextEditor. The actual context menu in both these cases displays at the insertion point, not at the touch location, which I believe is the correct/usual behavior iOS users expect. Let me know if you agree that:

  1. You also see the context menu in the MarkupWKWebView show up when there is no focus/caret in the MarkupEditorView/MarkupWKWebView.
  2. The lack of context menu when the long press is in the middle of the view when the caret is visible and focus is set on the MarkupWKWebView is the issue to be solved.
  3. The location of the context menu itself is not part of the issue.

The reason I'm asking about the 3rd item is because the picture you initially posted had the "Paste" menu in the middle while the caret is at the upper left, and I can't make that happen.

struct ContentView: View {
    @State private var demoHtml: String = "<p><br></p>"
    @State private var demoText: String = ""
    
    var body: some View {
        VStack {
            MarkupEditorView(html: $demoHtml)
                .border(Color.gray)
            TextEditor(text: $demoText)
                .border(Color.gray)
        }
    }
}

@stevengharris
Copy link
Owner

FYI, you can literally see why it's having problems by commenting-out the focus entry in markup.css so that the focus ring shows...

/*
#editor:focus {
    outline: none !important;
}
*/

@JohnKuan
Copy link
Contributor Author

  1. You also see the context menu in the MarkupWKWebView show up when there is no focus/caret in the MarkupEditorView/MarkupWKWebView.

Yes, regardless of the focus of the MarkupWKWebView, it should show up when the appropriate gesture is applied.

  1. The lack of context menu when the long press is in the middle of the view when the caret is visible and focus is set on the MarkupWKWebView is the issue to be solved.

Yes. This is the primary issue I am raising. I think the context menu should be presented at the carat, regardless of the touch location.

  1. The location of the context menu itself is not part of the issue.

Yes. I agree, the location of the context menu should not be at the touch location. The arrow tip on the context menu will be misleading in this scenario. My attempt to simulate a long press onto the MarkupWKWebView, resulted it to be shown in that way in my screenshot earlier.

This is what I was experimenting on iOS Notes App. For the long press, it has a magnifier focus showing the carat and the carat is taking the horizontal position of the carat. Upon releasing the hold, the carat will go to the word.

RPReplay_Final1681800652.MP4

@stevengharris
Copy link
Owner

stevengharris commented Apr 18, 2023

@JohnKuan Can you try adding padding-block: 0 100%; to the #editor:focus setting in markup.css and see if the behavior matches expectations. It should look like this:

#editor:focus {
    outline: none !important;
    padding-block: 0 100%;
}

@stevengharris
Copy link
Owner

stevengharris commented Aug 19, 2023

I merged a PR to apply the fixes discussed in this issue (which automatically closes the issue) but if it doesn't fix it, feel free to reopen it or file a new one. @JohnKuan

@JohnKuan
Copy link
Contributor Author

Hi @stevengharris this fixes the issue. Thanks!

@stevengharris
Copy link
Owner

@JohnKuan The fix here was causing issues for me on Mac Catalyst, where I am auto-sizing the WKWebView frame on the Swift side based on the contents. Although I found a way to get the "true" height of the WKWebView's contents without accounting for the padding, the view itself seems to take into account the padding when determining its scroll height. This in turn caused me issues when I wanted to adjust the WKWebView frame height as I edited. Anyway, if you have a chance, can you check out the change in #145 and make sure it still works for you. I don't think it will introduce any issues, but I'd like to double-check. Thx.

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 a pull request may close this issue.

2 participants