Feature/finished/iia 2356 copy publicid to clipboard by jdsmithsos · Pull Request #587 · ikmdev/komet

jdsmithsos · 2025-07-16T15:36:22Z

Jira ticket: https://ikmdev.atlassian.net/browse/IIA-2356

Summary of changes:

Added the new SVG for Copy to Clipboard icon to SVGConstants
Created a custom control PublicIDControl and the PublicIDSkin
Provided tooltip for the new Copy to Clipboard button
When the Public ID is hovered over the new Copy to Clipboard button appears, per the Acceptance Criteria in the ticket

Note: The library in Scene Builder needs to be updated as there is a new custom control used in the fxml and a new SVGConstants value

Before changes: There was not a Copy to Clipboard button

After changes:

Concept

Hovering over Public ID

Hovering over Copy to Clipboard button

Pattern

Hovering over Public ID

Hovering over Copy to Clipboard button

Semantic

Hovering over Public ID

Hovering over Copy to Clipboard button

TextField Copy

TextField select and copy is still supported.

Copied Values From Clipboard

Here are the values copied from Komet to Notepad++

…lipboard

dukke

1 - Can you make the button only visible when the mouse is hover the id field?
2 - could you make the button smaller and with no background?
3- Can you make it a custom control so we can reuse in the various places (without duplicating code)?

Thanks!

…sed for concept, pattern, semantic

dukke

@jdsmithsos
Great work. With the new custom control we can reuse and avoid duplicate code.
Left a few comments...

dukke · 2025-07-17T16:53:10Z

kview/src/main/java/dev/ikm/komet/kview/controls/skin/PublicIDSkin.java

+
+        // the SVG graphic for the copy to clipboard icon
+        var svgPath = new SVGPath();
+        svgPath.setContent(SVGConstants.COPY_TO_CLIPBOARD_SVG_PATH);


Is this one of those SVG that we can't set through CSS?

Is this one of those SVG that we can't set through CSS?

Yes, it does not render correctly in css.

dukke · 2025-07-17T16:54:11Z

kview/src/main/java/dev/ikm/komet/kview/controls/skin/PublicIDSkin.java

+///
+/// The publicIdLable and copyToClipboardButton are contained within an HBox, where the HBox is configured
+/// to handle mouse enter and exit events which is used to show and hide the copyToClipboardButton.
+public class PublicIDSkin implements Skin<PublicIDControl>  {


Small detail, you can derive from the SkinBase class (which implements Skin). You then don't need some of the methods since SkinBase takes care of it.

I tried extending SkinBase, but the control does not render at all.
I looked at the Javadoc for SkinBase, and the getNode() method is final. But there isn't a way to set the node. Is it derived from the getSkinnable() control? Don't know.
I think that is the problem, there is not a node value for getNode(), so it can't render the control.

@jdsmithsos so the way you do it is that you call getChildren() and add the node, or nodes you'd want, there. SkinBase is like a Region.
You don't need to override getNode or getSkinnable.

dukke · 2025-07-17T16:58:29Z

kview/src/main/java/dev/ikm/komet/kview/controls/skin/PublicIDSkin.java

+public class PublicIDSkin implements Skin<PublicIDControl>  {
+
+    /// The spacing between children in the HBoxes used in this Skin
+    private static final double HBOX_SPACING = 4.0;


Can we possibly have this UI details defined in the CSS?

Now using css, where the hbox spacing and the label font family and size are set

dukke · 2025-07-17T16:59:31Z

kview/src/main/java/dev/ikm/komet/kview/controls/skin/PublicIDSkin.java

+    private Subscription subscription;
+
+    /// The current public id property value, as received in the subscription listener
+    private String identifier;


Small detail, some of these fields can be made final.

dukke · 2025-07-17T17:01:58Z

kview/src/main/java/dev/ikm/komet/kview/controls/skin/PublicIDSkin.java

+        });
+
+        // initially hide the button
+        copyToClipboardButton.setVisible(false);


Can you also set the managed property to false when visibility is false and true when it's otherwise?
You can even potentially bind the 2 properties.
Reason is, if managed is false, JavaFX won't lose time laying out the Node (if it's not visible than we don't even need to do the lay out for it in this case)

Can you also set the managed property to false when visibility is false and true when it's otherwise? You can even potentially bind the 2 properties. Reason is, if managed is false, JavaFX won't lose time laying out the Node (if it's not visible than we don't even need to do the lay out for it in this case)

I will try. I originally coded with adding and removing the button as a child, but that was causing the layout to change to where the height was shorter without the button in the layout. So I switched to show and hide instead, which leaves the HBox height the same. I'm thinking if hidden and unmanaged, the HBox height will adjust just like it did when the button was removed from the layout.

I tried using setManaged(), but the height size change is the same as removing and adding the child button.
One workaround is to explicitly set the pref, min, max height (say to 30) which works without resizing the height.
But doing that will mean that if other components change within the HBox, such as font size change, icon size change, then the arbitrary height size of 30 will have to be adjusted to some other value.
Rather than doing all of this, I think that it is safer to let the layout height be automatically calculated with changing the visibility.

@jdsmithsos You can override computeMinWidth and/or computeMinHeight on the skin class so that the height or width is always a minimum value - the value that you return from these methods.
Toggling the managed property is better than adding and removing nodes while they're live in the scene. Doing that can have performance implications which can be serious implications depending on the number of nodes you remove/add.

side note there's also computePref... methods, etc

@jdsmithsos You can override computeMinWidth and/or computeMinHeight on the skin class so that the height or width is always a minimum value - the value that you return from these methods. Toggling the managed property is better than adding and removing nodes while they're live in the scene. Doing that can have performance implications which can be serious implications depending on the number of nodes you remove/add.

side note there's also computePref... methods, etc

@dukke I could not get the SkinBase to work. SkinBase has the compute* methods that can be overridden.

Can you also set the managed property to false when visibility is false and true when it's otherwise? You can even potentially bind the 2 properties. Reason is, if managed is false, JavaFX won't lose time laying out the Node (if it's not visible than we don't even need to do the lay out for it in this case)

done. I got the SkinBase working, now setting fx-pref-height and fx-min-height in css, also added setManaged() used alongside of setVisible()

dukke · 2025-07-17T17:03:04Z

kview/src/main/java/dev/ikm/komet/kview/controls/PublicIDControl.java

+
+    public SimpleStringProperty publicIdProperty() {
+        return publicId;
+    }


Can you also add the setter and the getter for the publicId property:
void setPublicId(String)
String getPublicId()

dukke · 2025-07-17T17:04:33Z

kview/src/main/java/dev/ikm/komet/kview/mvvm/view/details/DetailsController.java

+        idList.addAll(DataModelHelper.getIdsToAppend(viewCalculator, entityFacade.toProxy()));
+        String idStr = String.join(", ", idList);
+
+        identifierControl.publicIdProperty().setValue(idStr);


if you add the setter you can just call the setter here and on other places in the code.

dukke · 2025-07-17T17:09:05Z

kview/src/main/resources/dev/ikm/komet/kview/mvvm/view/genediting/genediting-details.fxml

+                                       </SVGPath>
+                                    </graphic>
+                                    <tooltip><Tooltip text="Copy to clipboard" /></tooltip>
+                                 </Button>


Do we still need this here? Since we have the custom control we can now use it here instead...

That is contained within commented out fxml, so I deleted the comments, no longer needed.

dukke · 2025-07-17T17:10:02Z

kview/src/main/resources/dev/ikm/komet/kview/mvvm/view/pattern/pattern-details.fxml

+                                          </SVGPath>
+                                       </graphic>
+                                       <tooltip><Tooltip text="Copy to clipboard" /></tooltip>
+                                    </Button>


we don't need this anymore since we have the custom control....

carldea · 2025-07-17T17:32:02Z

Nice work @jdsmithsos this is really shaping up! I do like @dukke 's suggestions.
@jdsmithsos Can you verify with the TE team the public id with SnomedCT or Loinc data. Because components can have what is called identifier semantics. so it can show more than the public id hash string.

…lipboard # Conflicts: # kview/src/main/java/dev/ikm/komet/kview/mvvm/view/pattern/PatternDetailsController.java # kview/src/main/resources/dev/ikm/komet/kview/mvvm/view/pattern/pattern-details.fxml

dukke

@jdsmithsos
Replied to your comment.

Thanks! :)

dukke · 2025-07-18T12:52:12Z

kview/src/main/java/dev/ikm/komet/kview/controls/skin/PublicIDSkin.java

+        });
+
+        // initially hide the button
+        copyToClipboardButton.setVisible(false);


@jdsmithsos You can override computeMinWidth and/or computeMinHeight on the skin class so that the height or width is always a minimum value - the value that you return from these methods.
Toggling the managed property is better than adding and removing nodes while they're live in the scene. Doing that can have performance implications which can be serious implications depending on the number of nodes you remove/add.

side note there's also computePref... methods, etc

… properties

jdsmithsos · 2025-07-18T15:50:53Z

Nice work @jdsmithsos this is really shaping up! I do like @dukke 's suggestions. @jdsmithsos Can you verify with the TE team the public id with SnomedCT or Loinc data. Because components can have what is called identifier semantics. so it can show more than the public id hash string.

@carldea The new custom control takes a simple String, it does not build the string for the public id. This can be done, but separate from the implementation of the new custom control.

…ref-height and icon-size

carldea · 2025-07-18T18:54:32Z

kview/src/main/java/dev/ikm/komet/kview/controls/skin/PublicIDSkin.java

+/// - Copy to Clipboard button - when pressed, copies the Public ID text to the System clipboard
+///
+/// The publicIdLable and copyToClipboardButton are contained within an HBox, where the HBox is configured
+/// to handle mouse enter and exit events which is used to show and hide the copyToClipboardButton.


Can you convert to standard Javadoc comments?

This is standard Javadoc Markdown as of Java 23. I will convert to the traditional /** doc comments if you like.

Oh wow! Good call. I totally forgot about the new Java doc feature.
Please go for it! 😁👍🏻

carldea · 2025-07-18T19:55:50Z

kview/src/main/java/dev/ikm/komet/kview/controls/skin/PublicIDSkin.java

+public class PublicIDSkin extends SkinBase<PublicIDControl> {
+
+    /// The root Node for the Skin
+    private final HBox rootHBox = new HBox();


Can we not use three forward slashes for comments?

carldea · 2025-07-18T19:57:57Z

kview/src/main/java/dev/ikm/komet/kview/controls/skin/PublicIDSkin.java

+    }
+
+    /// Copy the Public Identifier UUID String value to the System Clipboard
+    private void copyToClipboard() {


Can we use standard Javadoc comments for the method?

/** * comments go here. */

@carldea done

…itional Javadoc comments

carldea · 2025-07-19T00:19:30Z

Nice work @jdsmithsos this is really shaping up! I do like @dukke 's suggestions. @jdsmithsos Can you verify with the TE team the public id with SnomedCT or Loinc data. Because components can have what is called identifier semantics. so it can show more than the public id hash string.

@carldea The new custom control takes a simple String, it does not build the string for the public id. This can be done, but separate from the implementation of the new custom control.

No problem. I like how you did it. More general purpose use. The user of the custom can easily update the textProperty(). 👍🏻

…lipboard

dukke

Looking good... it's getting close... just added some more comments about some details.

Thanks for sharing the new javadoc 3 slashes syntax, didn't know about that.

dukke · 2025-07-21T11:47:38Z

kview/src/main/java/dev/ikm/komet/kview/controls/skin/PublicIDSkin.java

+ * The publicIdLable and copyToClipboardButton are contained within an HBox, where the HBox is configured
+ * to handle mouse enter and exit events which is used to show and hide the copyToClipboardButton.
+ */
+public class PublicIDSkin extends SkinBase<PublicIDControl> {


The convention is to have the same name of the control with a "Skin" appended. So here it would be "PublicIDControlSkin"

dukke · 2025-07-21T11:53:26Z

kview/src/main/java/dev/ikm/komet/kview/controls/skin/PublicIDSkin.java

+        Tooltip.install(publicIdLabel, publicIdTooltip);
+
+        rootHBox.getStyleClass().add("public-id");
+        rootHBox.getStylesheets().add(PublicIDControl.class.getResource("public-id.css").toExternalForm());


in the "Control" class there is a method you can override:
"public String getUserAgentStylesheet()"

In that method you can return the stylesheet of the control.

If you then want CSSFX to pick it up you can add that the useragentstylesheet to the scene's stylelsheets list (checkout the other control's we've done implementation for reference).

now using getUserAgentStylesheet() in PublicIDControl

dukke · 2025-07-21T11:58:10Z

kview/src/main/java/dev/ikm/komet/kview/controls/skin/PublicIDSkin.java

+        svgPath.getStyleClass().add("copy-to-clipboard");
+
+        copyToClipboardButton.setGraphic(svgPath);
+        copyToClipboardButton.getStyleClass().add("add-pencil-button");


Can we give this node a styleclass for this specifc control. Example: "copy-button".

Some comments on aesthetics:
1- could we have the button be dark grey colored?
2 - could we have the background color of the hover effect be light gray and with rounded corners? Also with a bit less padding?
3 - can we have the copy button be just a bit smaller?

Can we give this node a styleclass for this specifc control. Example: "copy-button".

Some comments on aesthetics: 1- could we have the button be dark grey colored? 2 - could we have the background color of the hover effect be light gray and with rounded corners? Also with a bit less padding? 3 - can we have the copy button be just a bit smaller?

I have reached out to Jieun and Gulnaz to get the specifics of what you are requesting. I would rather have an official Figma wireframe and descriptions than have us guessing what it should look like.

@jdsmithsos as discussed, I think we can do these small tweaks ourselves and not overburden Amplified.
I've done that in the past where I've designed some of the things I've implemented. Thanks :)

…selected and copied using keyboard ^C

…tUserAgentStylesheet() in PublicIDControl to provide the control stylesheet

…y theme as requested

dukke

Looks great. I believe this will be nice to have!
It will allow the user to just click the button to copy instead of having to select with the mouse and then right click and then copy which can be error prone and takes more steps.

…lipboard

jdsmithsos added 3 commits July 16, 2025 06:45

IIA-2356 adding copy to clipboard button for public id

46a8c5c

Merge branch 'main' into feature/finished/IIA-2356-copy-publicid-to-c…

00b8a6a

…lipboard

IIA-2356 updates for the public id copy to clipboard

e30ade4

jdsmithsos requested review from bhharsh13, carldea, dholubek, dukke and swaroopsalvi July 16, 2025 15:36

dukke requested changes Jul 16, 2025

View reviewed changes

jdsmithsos added 2 commits July 16, 2025 12:26

IIA-2356 back to where I was but now using a custom control that is u…

2f740b9

…sed for concept, pattern, semantic

IIA-2356 added Javadoc, adjusted the SVG icon size to be smaller

762cf6e

jdsmithsos requested a review from dukke July 17, 2025 12:12

jdsmithsos marked this pull request as ready for review July 17, 2025 12:13

dukke requested changes Jul 17, 2025

View reviewed changes

jdsmithsos added 2 commits July 17, 2025 12:49

IIA-2356 changes per PR review feedback

fb1586a

Merge branch 'main' into feature/finished/IIA-2356-copy-publicid-to-c…

d4c85d7

…lipboard # Conflicts: # kview/src/main/java/dev/ikm/komet/kview/mvvm/view/pattern/PatternDetailsController.java # kview/src/main/resources/dev/ikm/komet/kview/mvvm/view/pattern/pattern-details.fxml

dukke requested changes Jul 18, 2025

View reviewed changes

IIA-2356 now using css to define the configurable custom control skin…

af07f99

… properties

jdsmithsos requested a review from dukke July 18, 2025 15:49

IIA-2356 got the skin working now with SkinBase, using more css for p…

21ef6df

…ref-height and icon-size

carldea reviewed Jul 18, 2025

View reviewed changes

IIA-2356 converted from new Java 23 Markdown Javadoc comments to trad…

d813248

…itional Javadoc comments

jdsmithsos requested a review from carldea July 18, 2025 20:47

Merge branch 'main' into feature/finished/IIA-2356-copy-publicid-to-c…

3fd0561

…lipboard

dukke requested changes Jul 21, 2025

View reviewed changes

IIA-2356 converted back to the new Markdown Javadoc

8ad2331

jdsmithsos requested a review from dukke July 21, 2025 13:45

jdsmithsos added 4 commits July 21, 2025 09:01

IIA-2356 renamed to PublicIDControlSkin

0dd23e5

IIA-2356 changed the Public ID Label to TextField so the text can be …

6635780

…selected and copied using keyboard ^C

IIA-2356 renamed public-id.css to public-id-control.css, now using ge…

d436cdf

…tUserAgentStylesheet() in PublicIDControl to provide the control stylesheet

IIA-2356 adjusted L&F of the Copy to Clipboard button to the dark gre…

24af720

…y theme as requested

dukke approved these changes Jul 22, 2025

View reviewed changes

jdsmithsos added 3 commits July 22, 2025 12:02

IIA-2356 adjusted the hover and pressed colors

9e1dfc7

Merge branch 'main' into feature/finished/IIA-2356-copy-publicid-to-c…

f5dbcb1

…lipboard

Merge branch 'main' into feature/finished/IIA-2356-copy-publicid-to-c…

3dc0d20

…lipboard

jdsmithsos merged commit 7fb9a44 into ikmdev:main Jul 24, 2025
7 checks passed

jdsmithsos deleted the feature/finished/IIA-2356-copy-publicid-to-clipboard branch July 25, 2025 17:43

Conversation

jdsmithsos commented Jul 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Concept

Pattern

Semantic

TextField Copy

Copied Values From Clipboard

Uh oh!

dukke left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dukke left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dukke Jul 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

carldea commented Jul 17, 2025

Uh oh!

dukke left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jdsmithsos commented Jul 18, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

jdsmithsos commented Jul 16, 2025 •

edited

Loading

dukke left a comment •

edited

Loading

dukke Jul 18, 2025 •

edited

Loading

dukke Jul 21, 2025 •

edited

Loading