Discussion: PropertySheet: Leave text fields for read-only property-items with setDisabled(false) and only setEditable(false)

[vatbub]

Hi there!
I would like to open a small discussion whether read-only property items in a PropertySheet that use a text-field based editor should only set setEditable(false) and not set setDisabled(true), as it is currently done.

The advantage of the current UX is that it is very clear to the user that a given property is read-only. However, in my case, I would like the user to be able to select the contents of the text field to copy them into the clipboard, which is impossible when setting setDisable(true).

I am currently working on a fix for this issue in a fork but would like other opinions before creating a PR.

Cheers :)

[danielpeintner]

I think 2 aspects would be nice:

• being able to copy the content even if one cannot modify it
• still seeing visually that the content is not modifiable (as of now with the gray background, compared to white)

[eugener]

Sounds like a good addition!
Existing functionality should be preserved though to make sure new changes to do not break existing UIs. This can be done by putting new behavior behind the "flag" which is off by default. For example, COPY_READONLY_CONTENT. If the flag is set, we turn off field's disabled state and emulate disabled style.
Even though it is possible just to add this new functionality, I worry about existing apps as the library been out-there for almost 10 years now.
PRs are always welcome! :)

[vatbub]

I tried the approach to be able to copy it without changing the visual style by creating a custom editor and have a copy-button right next to the text field and have it all wrapped in an HBox. The issue is though that AbstractPropertyEditor just disables the entire editor control (so in that case the entire HBox) and therefore also the button.

As a quick fix, I did exactly what I mentioned in the question (on my private fork of the library) and only set setEditable(false), but the entire lack of visual indication that the property is not editable is a little weird, IMO. So, at the very least, we need to emulate the disabled style.

However, I am not entirely sure if we really need a new flag if we just emulate the disabled style, but I am also not fully aware of the possible consequences of such a change given the age of the library. If we were to add a new flag, where would you suggest adding it? In the Property.Item interface?

[eugener]

To be honest, I'm not sure either - I just worry about the consequences to existing apps.
As for flags, I would do it as a getFlag/setFlag static methods on PropertySheet itself to simplify developer access and provide an ability to customize any part of control. Those would get and set values for specified flags into internal static map, which than can be checked by internal code to produce proper behavior. This assumes of course that we want this behavior to be uniform for all instances of the control within the application.

[eugener]

In theory, Node's getProperties/setProperties can be used but that only allows us to customize specific instance of the control

[danielpeintner]

I am currently working on a fix for this issue in a fork but would like other opinions before creating a PR.

Hi @vatbub have you been able to work on it?

[vatbub]

TBH it's been a while since I worked on that and I forgot what I ended up doing. I'll check tomorrow.

[danielpeintner]

Thanks, any feedback is welcome. If there is any starting point I am interested in continuing the work.