feat: add Clipboard text/html write API#23615
Conversation
Test Results1 210 files - 203 1 210 suites - 203 1h 15m 10s ⏱️ - 8m 47s For more details on these failures, see this check. Results for commit c07ac1f. ± Comparison against base commit 9b240a3. ♻️ This comment has been updated with latest results. |
f461868 to
f00593a
Compare
|
|
I did a test with the view at https://gist.github.com/mcollovati/0090b03502eacc0bf96a4336612a2424 Seems to work fine, except for EDIT: update the gist with a workaround for copy/cut listeners |
Adds a new use-cases module mirroring the geolocation/signals layout, covering the Clipboard API requirements from vaadin/platform#8759 and the Flow PR vaadin/flow#23615: UC1 — copy static text on click (with success/error callbacks) UC2 — copy current value of a component UC3 — copy image UC4 — paste text and HTML UC5 — paste files UC6 — copy via a context-menu item UC7 — detect availability and degrade gracefully
|
Tested with the https://clipboard-cases.fly.dev - some use cases don't work:
|
|
There was an issue where the newest version of the use cases app wasn't auto deployed: vaadin/use-cases#235. I think most of the mentioned issues are not in that app but it still makes sense to check |
mshabarov
left a comment
There was a problem hiding this comment.
First round of review, focus on implementation.
| public byte[] getData() { | ||
| return data; | ||
| } |
There was a problem hiding this comment.
The method's docs say do not modify, but the array is returned as-is. A copy could be returned or an immutable object.
There was a problem hiding this comment.
Also makes sense to add getInputStream (get data as an input stream).
| * @throws NullPointerException | ||
| * if {@code target} or {@code listener} is {@code null} | ||
| */ | ||
| public static Registration addPasteListener(Component target, |
There was a problem hiding this comment.
If I do
Registration r1 = Clipboard.addPasteListener(target, l1);
Registration r2 = Clipboard.addPasteListener(target, l2); // overwrites attribute
r1.remove(); // removes attribute
So r2's listener still fires, but files never upload anymore because the attribute was removed.
Maybe we should throw in this case and support only one listener per target component?
| * @throws NullPointerException | ||
| * if any argument is {@code null} | ||
| */ | ||
| public static Registration addPasteListener(Component target, |
There was a problem hiding this comment.
Similar issue here - two overloads of addPasteListener cannot be used simultaneously because they share the same attribute.
| * @throws NullPointerException | ||
| * if {@code trigger} is {@code null} | ||
| */ | ||
| public static ClipboardCopy copyOnClick(Component trigger, String text) { |
There was a problem hiding this comment.
Similar problem as for paste listener:
ClipboardCopy h1 = Clipboard.copyOnClick(btn, "A");
ClipboardCopy h2 = Clipboard.copyOnClick(btn, "B"); // h1 silently superseded
h1.remove(); // tears down h2's handler
// h2 looks active but clicks no longer copy anything
because one slot per element in used in JavaScript so these two objects/handlers have dependencies to each other.
| UploadHandler uploadHandler, @Nullable PasteState pasteState, | ||
| SerializableConsumer<ClipboardEvent> listener) { | ||
| Element element = target.getElement(); | ||
| element.setAttribute("__clipboard-paste-upload", uploadHandler); |
There was a problem hiding this comment.
Minor: custom attributes are usually data-*. Using data-clipboard-paste-upload would be more idiomatic and won't surprise anyone inspecting the DOM, this convention is used somewhere else in the Flow already. The double-underscore convention seems borrowed from the __clipboardText property (which is fine because it's a JS property, not an HTML attribute).
| state.clickHandler = async () => { | ||
| const src = (state.image as HTMLImageElement | undefined)?.src; | ||
| if (!src) { | ||
| return; |
There was a problem hiding this comment.
Maybe worth to log an error/debug message with concole.debug here if an image has no src ?
mshabarov
left a comment
There was a problem hiding this comment.
Several API-related comments
| * @throws NullPointerException | ||
| * if {@code trigger} or {@code imageSource} is {@code null} | ||
| */ | ||
| public static ClipboardCopy copyImageOnClick(Component trigger, |
There was a problem hiding this comment.
Text copy variant has no "text" in the name. Every new content type forces a new top-level method (e.g. copyHtmlOnClick, copyFileOnClick). This can be improved by grouping copy sources into categories:
Clipboard.on(button).copy(String);
Clipboard.on(button).copyValueOf(HasValue);
Clipboard.on(button).copyImageOf(Image);
also taking into account that a trigger is always needed.
There was a problem hiding this comment.
Feels to me that we can simply put an Image as an argument instead of Component.
But what about IFrame and HtmlObject, shall we allow them to copy their image to clipboard?
| */ | ||
| public static ClipboardCopy copyImageOnClick(Component trigger, | ||
| Component imageSource, @Nullable Command onSuccess, | ||
| @Nullable Command onError) { |
There was a problem hiding this comment.
onError is just a command without any information about an error. This could give error code, clipboard availability, exception type, message.
| * registered for success/error callbacks. Idempotent. | ||
| */ | ||
| @Override | ||
| public void remove() { |
There was a problem hiding this comment.
I couldn't find a good example of unregistering from "copy to clipboard" events - these are permanent in most of the cases I think until they need an explicit clean up upon detach. The use-case project doesn't use this at all, that's a sign.
On the other hand, this method looks very useful
var copy = Clipboard.copyOnClick(button, link);
// later, when the share URL is regenerated server-side:
copy.setText(newLink);
but this isn't used in the use cases either.
| public byte[] getData() { | ||
| return data; | ||
| } |
There was a problem hiding this comment.
Also makes sense to add getInputStream (get data as an input stream).
| * cut events typically carry only text and HTML — files are paste-only on the | ||
| * browser side. | ||
| */ | ||
| public final class ClipboardEvent implements Serializable { |
There was a problem hiding this comment.
Could replace hasXyz() and getXyz() methods with Optional<XyzType> xyz() or Optional<XyzType> getXyz(), e.g.Optional<String> text().
| * @throws NullPointerException | ||
| * if {@code target} or {@code listener} is {@code null} | ||
| */ | ||
| public static Registration addPasteListener(Component target, |
There was a problem hiding this comment.
I noticed in the use cases examples that the drop target uses dropZone.getElement().setAttribute("tabindex", "0"); to make it focusible otherwise clipboard paste won't work, maybe we should add it automatically?
| * this value is essentially never observed in practice; once a real value | ||
| * has arrived, the signal never returns to {@code UNKNOWN}. | ||
| */ | ||
| UNKNOWN |
There was a problem hiding this comment.
Could we consider merging UNSUPPORTED and UNKNOWN into one (and maybe turn the enum into boolean) as I assume a code in the projects would perhaps always be like:
if (clipboardAvailability == AVAILABLE) {
myComponent.setEnabled(true);
} else {
logger.trace("Clipboard is not available for user " + user);
myComponent.setEnabled(false);
}
| public static ClipboardCopy copyOnClick(Component trigger, | ||
| Component source) { |
There was a problem hiding this comment.
Can/should this be
copyOnClick(Component trigger, HasValue<?, ?> source)
| * @throws NullPointerException | ||
| * if {@code trigger} or {@code source} is {@code null} | ||
| */ | ||
| public static ClipboardCopy copyOnClick(Component trigger, |
There was a problem hiding this comment.
Does it make sense to provide an overload that would allows to give a function
public static <C extends Component> ClipboardCopy copyOnClick(
Component trigger, C source,
SerializableFunction<C, String> textProvider)
so that I can set up a custom text
TextField customer = new TextField("Customer ID");
Button copy = new Button("Copy link");
Clipboard.copyOnClick(copy, customer,
field -> "https://crm.example.com/customer/" + field.getValue());
|
What if we introduce
|
|
About the shortcut trigger, I'm not sure we should provide it for clipboard, even though the trigger itself makes sense for other features. Two reasons: browsers react to Ctrl+C/Cmd+C already when you have selection on the page; a better way is to select and focus an element or text in an element that then could be copied by browser native shortcut. |
| /** Event type identifier for paste events. */ | ||
| public static final String PASTE = "paste"; | ||
| /** Event type identifier for copy events. */ | ||
| public static final String COPY = "copy"; | ||
| /** Event type identifier for cut events. */ | ||
| public static final String CUT = "cut"; |
There was a problem hiding this comment.
These could be enumeration values?
|
|
||
| private final String type; | ||
| private final @Nullable String text; | ||
| private final @Nullable String html; |
There was a problem hiding this comment.
I wonder how html is more special than just a text string? Maybe we can remove it?
16cb109 to
79fdb13
Compare
|
Rewrote this PR to be much smaller and more focused - now only handles text/html copying and we can do the rest in followup PRs |
|
Public surface (com.vaadin.flow.component.clipboard):
Clipboard.on(ClickNotifier) // click trigger sugar
Clipboard.on(Trigger) // any custom trigger
ClipboardBinding verbs (each in fire-and-forget + observed flavours):
copyTextFrom(String | HasValue+Component | Action.Input<String>)
copyHtmlFrom(String | Action.Input<String>)
copyFrom(ClipboardContent) // multi-format ClipboardItem
The observed flavours take onSuccess (SerializableRunnable) and onError
(SerializableConsumer<String>) consumers, dispatched on the UI thread by
PromiseAction. Both are required in the observed form — pass () -> {} or
err -> {} to opt out of one.
Internals (com.vaadin.flow.component.trigger.internal, alongside the
framework's other actions because JsBuilder is package-private):
- ClipboardCopyAction extends PromiseAction; emits
`navigator.clipboard.write([new ClipboardItem({...})])` with any
combination of text/plain and text/html slots.
Tests verify the generated JsFunction body (e.g. "text/plain":"Hello",
$0["value"]), the trigger entry points (click vs. custom DOM event), and
that empty ClipboardContent rejects.
Image copy and clipboard availability are intentionally split into
follow-up commits; active reads and paste/copy/cut event capture are
not yet exposed (they need a typed server-callback primitive queued for
trigger-step3).



Public surface (com.vaadin.flow.component.clipboard):
Clipboard.on(ClickNotifier) // click trigger sugar
Clipboard.on(Trigger) // any custom trigger
ClipboardBinding verbs (each in fire-and-forget + observed flavours):
copyTextFrom(String | HasValue+Component | Action.Input)
copyHtmlFrom(String | Action.Input)
copyFrom(ClipboardContent) // multi-format ClipboardItem
The observed flavours take onSuccess (SerializableRunnable) and onError
(SerializableConsumer) consumers, dispatched on the UI thread by
PromiseAction. Both are required in the observed form — pass () -> {} or
err -> {} to opt out of one.
Internals (com.vaadin.flow.component.trigger.internal, alongside the
framework's other actions because JsBuilder is package-private):
navigator.clipboard.write([new ClipboardItem({...})])with anycombination of text/plain and text/html slots.
Tests verify the generated JsFunction body (e.g. "text/plain":"Hello",
$0["value"]), the trigger entry points (click vs. custom DOM event), and
that empty ClipboardContent rejects.
Image copy and clipboard availability are intentionally split into
follow-up commits; active reads and paste/copy/cut event capture are
not yet exposed (they need a typed server-callback primitive queued for
trigger-step3).