Add a button to the KMessageWidget to make it really easy to copy the link to the clipboard.
CCBUG: 392722
We can still make this behavior automatic in the future if we want to, which would resolve 392722. Hopefully this should at least make it a bit easier.
| No Linters Available |
| No Unit Test Coverage |
Idea is good, code needs some work.
mSharedImageURL needs resetting and
mMessageWidget->removeAction(mCopyUrlToClipboardAction); needs to happen in more places
From what I can tell reading this the same message widget is used for errors and when you normally copy an image.
So with this patch as-is if you take a screenshot and share it, then take another image and hit copy to clipboard it will copy the old link instead of the image and show an incorrect button.
Thanks for the patch. I don't think having a dedicated button is strictly needed, as I'd rather turn on copying the link to the clipboard by default and communicate that through an inline message. Also, you can right-click on the link.
Still, for the quit-after-copy case this probably makes sense, so let's go with it (and I'm tired of arguing with you or David, TBH).
| src/Gui/KSMainWindow.cpp | ||
|---|---|---|
| 66 | Isn't this equivalent to the default constructor of QString? | |
| 194 | Please also set a more appropriate tooltip than the default one. | |
| 194 | Title case. | |
| 195 | Is this needed? | |
| 348–349 | Moving code to the else block by indentation does not work in C++. Another reason why we should always use curly braces. | |
| 366 | Hm, I don't really like how this is reusing sendToClipboard and then has to have two code paths based on mSharedImageURL.isEmpty(). This should be a dedicated slot. Let me refactor things a bit to make this easier for you and reduce code duplication. Won't take me long, promised (probably tonight). | |
| 371 | QApplication::clipboard(), to make it consistent with the implementation in doCopyToClipboard(). | |
| 372 | You probably won't need this after my refactoring, but for the record, there is QString::clear(). | |
change the string to just "Copy link", ommitting the "to clipboard" part.
That would be appreciated (and we could even think about doing the same for the button below, to keep consistency, e.g. Copy Screenshot?).
Edit: Hm, coming back I don't really like Copy Screenshot. Maybe just Copy? In every app's menu it's simply called "Copy", it should be clear by the resulting notification and lack of "..." that it copied directly to the clipboard anyway.
Also, Copy and Copy Link are different enough.
Alternatively, always copy the link automatically (i.e. no extra button, or rather only a delete button) and change the message accordingly.
I'm thinking of abandoning this in favor of D13493: Auto-copy shared image link to clipboard. Any opinions?