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

New Clipboard Adoption - CMP-7402 #1796

Open
wants to merge 10 commits into
base: jb-main
Choose a base branch
from
Open

Conversation

eymar
Copy link
Member

@eymar eymar commented Jan 24, 2025

Adopt new Clipboard interface on all targets.

New public api: It introduces some new public, platform-specific API: desktop, ios, web. See ClipEntry.
For now each target implements it separately - there is no common API except the trivial class. This class wraps the platform type representing the Clipboard content.

Fixes https://youtrack.jetbrains.com/issue/CMP-7402

Testing

  • added some basic unit tests. The extensive automated testing was not achieved here due to permission requirements (can't be easily granted automatically in the test environment)

This should be tested by QA

Release Notes

Features - Multiple Platforms

  • Adopt a new Clipboard interface with suspend functions, which work correctly on all targets including Web. The ClipboardManager was deprecated because it was not possible to correctly implement it for Web.

Asking for 3 reviewers to review the new public API for every platform.

@eymar eymar requested review from m-sasha, Schahen and ASalavei January 24, 2025 17:22
import java.awt.datatransfer.Transferable
import java.awt.datatransfer.UnsupportedFlavorException

actual typealias NativeClipboard = java.awt.datatransfer.Clipboard
Copy link
Member

@MatkovIvan MatkovIvan Jan 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exposing raw public typealias is a bad idea - it binds to AWT for desktop implementation without any options to compatibly change that.

Please consider changing that to extensions in platform sourcesets and removing this public API from common

PS sorry for late feedback, I had to do it in AOSP CL earlier. However, we still have an option to change this

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exposing raw public typealias is a bad idea - it binds to AWT for desktop implementation without any options to compatibly change that.

We can make it Any, similar to nativeEvent in PointerEvent:

cc @m-sasha Wdyt?

Please consider changing that to extensions in platform sourcesets

I'll see how it would look like.

and removing this public API from common

It can be only deprecated, because Android uses this approach already, and it's not epxerimental. Given other prioroties, working on a new proposal for this API won't happen soon.

assertNotNull(nativeClipboard)

runBlocking {
// Can't do much more asserts in a unit test. For more checks use a UI (instrumental) test
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Can't do much more asserts in a unit test. For more checks use a UI (instrumental) test
// Can't do much more asserts in a unit test. For more checks use an instrumented test

@OptIn(ExperimentalTestApi::class)
class UiKitPlatformClipboardTest {

// TODO: consider writing instrumental tests for Clipboard
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// TODO: consider writing instrumental tests for Clipboard
// TODO: consider writing instrumented tests for Clipboard


// TODO https://youtrack.jetbrains.com/issue/CMP-1260/ClipboardManager.-Implement-getClip-getClipMetadata-setClip
actual val clipMetadata: ClipMetadata
get() = TODO("ClipMetadata is not implemented. Consider using nativeClipboard")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It look list there is UTType as a ClipMetadata representation on iOS. Do you want to use it in a separate MR?
I'll keep this sample here just in case:

        val cString = CFStringGetCStringPtr(kUTTypeText, kCFStringEncodingUTF8)
        val string = NSString.stringWithCString(cString) as String
        println(string)
        val type = UTType.typeWithIdentifier(string)
        println(type)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The TODO was added before in skikoMain. But now one implementation in skikoMain won't work. So I copy/pasted the TODO too.
Yes, I believe it will be implemented in a separate MR.

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 this pull request may close these issues.

3 participants