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

Backup: copy-paste support with refactor of java side events handling #231

Closed
wants to merge 9 commits into from

Conversation

tpoisseau
Copy link
Contributor

with bubbles

Refs: #229

@tpoisseau tpoisseau linked an issue Oct 8, 2024 that may be closed by this pull request
Copy link

cloudflare-workers-and-pages bot commented Oct 8, 2024

Deploying openchemlib-js with  Cloudflare Pages  Cloudflare Pages

Latest commit: 5d1f3ff
Status: ✅  Deploy successful!
Preview URL: https://135763ca.openchemlib-js.pages.dev
Branch Preview URL: https://copy-paste-events-backup.openchemlib-js.pages.dev

View logs

@tpoisseau tpoisseau force-pushed the 229-improve-web-component branch 2 times, most recently from cd44ff7 to e548afe Compare October 11, 2024 09:27
@tpoisseau tpoisseau force-pushed the 229-improve-web-component branch from e548afe to 5d1f3ff Compare October 11, 2024 09:28
Comment on lines -77 to -78
ev.stopPropagation();
ev.preventDefault();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed so browser can propagate copy-paste events.
To interact with clipboard, browser need proper user interaction

Comment on lines +90 to +92
// copy-paste is handled by the clipboard event
if (isMenuKey(ev) && ev.key === 'c') return;
if (isMenuKey(ev) && ev.key === 'v') return;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

not triggering java events withfireKeyEvent in this case, avoid double copy-paste events.
And events triggered from keyboard can't work. Need clipboard events

public StereoMolecule pasteMolecule(boolean prefer2D, int smartsMode) {
Object test = null;
System.out.println("test: " + test.toString());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

test.toString() produce an error (test is null).

Theses errors was really annoying to implement copy-paste properly.

So I just changes theses method so it's a no-op

@tpoisseau
Copy link
Contributor Author

@targos I think it's working as expected, but I had to make lot of changes in Java side to implement copy-paste properly.

Do you agree on this approach ?
An other approach I can see is to do all the copy-paste system in JS side (and not propagate ctrl+c, ctrl+v events to java).

Actual approach is a bit hacky, (I also need to not propagate ctrl+c, ctrl+v to java). Doing in full JS will be hacky in the same way, and will be harder to replicate same java functionalities.

For me the best approach would be to refactor IClipboardHandler to support async operation (at least with callbacks), with an API which could be compatible with ClipboardEvent DataTransfer API.
I could manage the design of API and their implementations for JS side, but it will be hard for me to implement the java system (native) side.

Do you prefer I cut this PR into one for webcomponent and another one for copy-paste system ?

@targos
Copy link
Member

targos commented Oct 14, 2024

Yes, please split the PR. We always prefer to do many smaller PRs if it makes sense.

@tpoisseau tpoisseau changed the title fix: sync props (state) with attributes and propagate changes Backup: copy-paste support with refactor of java side events handling Oct 14, 2024
@tpoisseau
Copy link
Contributor Author

Please don't delete branch, backup for later works on copy-paste

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.

Improve web component
2 participants