Support clicking directly on a URL to open it

[trygveaa]

This allows you to click/press directly on a URL in the terminal view to
open it. It takes priority over opening the keyboard, so if you click on
a URL it is opened, and if you click anywhere else the keyboard opens
like before.

Currently, if the application in the terminal is tracking the mouse and
you click on a URL, both actions happen. The mouse event is sent to the
application, and the URL is also opened.

[agnostic-apollo]

Ah, finally completed it, eh! Congratz

Sorry for the delay, didn't get time to test. It seems to be working fine on my device without a mouse.
The issue that I see is that if the url doesn't fully cover the whole line and you click on the blank space on the right, it will still open the url. Is that solvable in some way or would be troublesome due to 5f71e3e?

Also should add a key in termux.properties to enable/disable this behaviour, since everybody may not want this and slower devices may be affected too. A lot of things have been added to termux recently and I wouldn't want to slow down the overall user experience, which may not be detectable on relatively faster test devices. Will have to decide whether it should be enabled by default or not though.

Maybe this should also only be triggered if ctrl key is down, like xfce4-terminal doesn't open urls on click, but requires a right click that shows the Open Link option once you hover over a url and it is highlighted and it doesn't have the blank space problem above. Would this also be helpful for mouse issues? What do you think?

Also regex Pattern should be compiled once and not on every touch. I will/can do this and add termux.properties key after the merge since otherwise there will be conflicts with my local master branch if you do it here. Already TermuxTerminalViewClient likely has them and I need to change it and TerminalView further to fix other stuff, so should merge before doing that.

Also it would be better if you added some comments in getWordAtLocation() to explain what that logic is doing so it's easier for the reader to understand and maintain.

Thanks

[trygveaa]

Sorry for the delay, didn't get time to test.

No worries, three days isn't that long :)

It seems to be working fine on my device without a mouse.
The issue that I see is that if the url doesn't fully cover the whole line and you click on the blank space on the right, it will still open the url. Is that solvable in some way or would be troublesome due to 5f71e3e?

Ah, I see what you mean, hadn't noticed that. I don't think the commit you're linking to matters for this. I'll take a look at it.

Also should add a key in termux.properties to enable/disable this behaviour, since everybody may not want this and slower devices may be affected too. A lot of things have been added to termux recently and I wouldn't want to slow down the overall user experience, which may not be detectable on relatively faster test devices. Will have to decide whether it should be enabled by default or not though.

Hm, not sure I see how it would be slowed down? It only runs if you tap the screen, and you don't typically do that very often for other things? But sure, we can have an option. I see below that you want to do this?

Maybe this should also only be triggered if ctrl key is down, like xfce4-terminal doesn't open urls on click, but requires a right click that shows the Open Link option once you hover over a url and it is highlighted and it doesn't have the blank space problem above. Would this also be helpful for mouse issues? What do you think?

I would hate that, since I want to be able to open URLs in the easiest way possible (I use IRC and Slack in Termux, so I open URLs all the time). But we could have an option for it.

Also regex Pattern should be compiled once and not on every touch. I will/can do this and add termux.properties key after the merge since otherwise there will be conflicts with my local master branch if you do it here. Already TermuxTerminalViewClient likely has them and I need to change it and TerminalView further to fix other stuff, so should merge before doing that.

Right, I just reused extractUrls without looking much at it.

Also it would be better if you added some comments in getWordAtLocation() to explain what that logic is doing so it's easier for the reader to understand and maintain.

Sure.

I'm on vacation now though, so will do this next week.

[agnostic-apollo]

Ah, I see what you mean, hadn't noticed that. I don't think the commit you're linking to matters for this. I'll take a look at it.

Thanks. If it's fixable, then great.

Hm, not sure I see how it would be slowed down? It only runs if you tap the screen, and you don't typically do that very often for other things? But sure, we can have an option. I see below that you want to do this?

I would hate that, since I want to be able to open URLs in the easiest way possible (I use IRC and Slack in Termux, so I open URLs all the time). But we could have an option for it.

Probably, slowing down may not be that much an issue, if regex is precompiled. But would be better to have an option in case someone doesn't want it.

Okay, then we can just leave it at directly opening on touch for now unless some issue arises in future for someone.

Right, I just reused extractUrls without looking much at it.

No worries.

Sure.

Thanks

I'm on vacation now though, so will do this next week.

Yeah, enjoy for now then. Will leave this for next release. I will refactor the extractUrls and add the termux.properties in this release, you can hook into it later.

[agnostic-apollo]

You can check 69bebb5 and 28b9f93 for how to hook.

[trygveaa]

Sorry for the very long delay here. I've now fixed the issue if you click on the blank space on the right, taken the option you added into use, added some comments and tests for getWordAtLocation and rebased on master.

Btw, one thing I noticed while testing this is that when you long press to select text, an offset is applied so the word on the line above where you pressed is selected. However when mouse tracking is enabled, this offset is not applied, so that means that different lines are affected when you click and when you long press when mouse tracking is enabled.

For URL opening I used the same code to get the position as the long press uses, so that uses the offset.

[agnostic-apollo]

No worries about the delay. Thanks for adding the comments and tests. The issues seem to be fixed.

I am not sure why that offset was added, might be because pressing with finger will have a larger touch area and we need to select bottom of line. Is that affecting behaviour for you?

Pull request looks good. I can merge it if you don't plan on changing anything further. I force pushed to use ShareUtils.openURL() instead of keeping duplicated code. The master branch will show the system chooser too as per dd952a9.

[trygveaa]

I am not sure why that offset was added, might be because pressing with finger will have a larger touch area and we need to select bottom of line. Is that affecting behaviour for you?

I'm not sure either. Well, it is affecting me in that I have to press a different place when I want to select/open url and when I want to interact with an application with mouse tracking. Not really related to this PR though, other than that I had to choose one of the behaviors when implementing it, and I chose the same as when selecting text because that fit better with the code. But as for the actual behavior I'm not sure I see how the offset is improving things.

Pull request looks good. I can merge it if you don't plan on changing anything further. I force pushed to use ShareUtils.openURL() instead of keeping duplicated code. The master branch will show the system chooser too as per dd952a9.

Great. No, I don't plan any more changes.

[agnostic-apollo]

I'm not sure either.

Well, neither the comment, nor the commit message (check top) when it was added in terminal-view has any detail for why. But it was actually added in jackpal source code for "Add a verical offset so you can see what you're selecting.". That was not fun to find, had to jump over multiple file moves!

So it has no significance in actual selection. If its indeed causing problem in selection with accuracy, then maybe it should be removed, unless someone objects.

Great. No, I don't plan any more changes.

If you wanna remove the offset, you can do it here, otherwise will merge.

[trygveaa]

If you wanna remove the offset, you can do it here, otherwise will merge.

Alright, I've removed the offset and changed the calculation of column number for selection/url to match the calculation used for mouse tracking. See commit 54bb83d for an explanation.

[agnostic-apollo]

Testing and working fine for both tapping and clicking. Users would need to fix their muscle memory and tap directly on the text since offset has now been removed, but shouldn't be too much of an issue. I think removing offset would also improve selecting text at the bottom of the terminal too.

Thanks for this!

[ghost]

will we get this update on playstore or f-droid version of termux app?

[ghost]

you can make a popup message which can ask users if they want to leave the app to open the url or not

"do you want to open the url?"
"yes" "no"

[agnostic-apollo]

will we get this update on playstore or f-droid version of termux app?

Playstore releases have been deprecated and haven't received an update for over an year.

https://github.com/termux/termux-app#google-play-store-deprecated

you can make a popup message

Can be added later. Also an option to view in browser or webview.

[Mark-Joy]

How does this feature work?
I tap on the url: https://google.com
Nothing happened except soft-keyboard is opened.

Click-Url.mp4

[agnostic-apollo]

Read changelog.

https://github.com/termux/termux-app/releases/tag/v0.118.0

[Mark-Joy]

app version 0.118
terminal-onclick-url-open=true in ~/.termux/termux.properties. termux-reload-settings was complete.
Still nothing when click on url.

Click-url1.mp4

[agnostic-apollo]

Exit termux app completely and start again. termux-reload-settings does not work on some devices, granting draw over apps permission may allow it to work.

[Mark-Joy]

Turns out that I had a space character after true value in ~/.termux/termux.properties: 😒😒
terminal-onclick-url-open=true[space]