Allow context menu to display options for both copy text and copy URL #3474
Conversation
Issue laurent22#3404 points out that on MacOS, right clicking on a link only displays the 'Copy' context menu item, instead of 'Copy Link'. It appears that MacOS selects the text while secondary clicking so the currently implemented conditional exits before realizing a link is also present. This change will allow context menu items for both copying text and the link URL. It would likely be cleaner to have ContextMenuItemType enum values in an array, and display options based on the items in the array, but I'm worried about unintended consequences of that big of a refactor since I don't really see tests around any of this.
|
I think your fix makes sense but I'm making a few changes related to the context menu in the UI Update branch so I think we'll have to go back to it once this branch is merged. |
|
Sounds good, thanks @laurent22. Feel free to just close this if you end up addressing it in that PR, or I can try to address any merge conflicts when that work is complete! |
|
@jbrr, I've rebase the pull request on the 1.2 release and as expected there are a few conflicts. Nothing to complex to solve at first sight, but could you try to fix them and see if your feature still works on macOS? |
|
@laurent22 sorry about that, I've been on vacation for the last 2 weeks and didn't bring my computer. I'll try to fix the conflicts and test it on my machine later this week or this weekend. |
|
It might be useful to add an option to convert webpage (from the link) to a note. |
|
@jbrr, in fact let's wait till version 1.3 to merge this as I expect it will bring more conflicts so may as well fix them all in one go. Sorry about that, your pull request arrives in the middle of various major changes so that makes things a bit complicated. |
|
@laurent22 no problem at all. Feel free to ping me again when things have calmed down a little bit. |
|
@jbrr, the latest changes are now on dev and should be stable, so if you'd like to fix the conflict I think we can merge your PR. |
|
@jbrr, I'll close this pull request for now, but feel free to resubmit if you'd like to fix it, or I may go back to it at some point too. |
Fixes #3404, kind of. It also adds a minor feature, which may not be desirable.
I'm not sure when this changed, but when a user on MacOS secondary clicks a link, it also selects the link text. It seemed like the least risky way to fix this was to add an extra option to the context menu. The new logic is, if text which is not part of a link is selected when the user secondary clicks, the context menu shows "Copy". If the user secondary clicks on a link without any selected text (presumably this is still possible on Windows and Linux, although I have not tested), the context menu will show "Copy Link Address". If the user secondary clicks on a link and has selected text, there will be two context menu options - "Copy" and "Copy Link Address". This will always be the case on MacOS, as far as I can tell.
I don't see any tests around any of this, but I'm happy to update them if that was an oversight on my part. I also don't have easy access to a Windows or Linux machine, so I've only tested on MacOS 10.15.5 Catalina, but this appears to work just fine there.
This is my first time attempting to contribute, apologies if I missed anything!