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

Make arbitration chat messages selectable #1326

Conversation

blabno
Copy link

@blabno blabno commented Feb 5, 2018

Fixes #1058

@blabno blabno requested a review from ManfredKarrer as a code owner February 5, 2018 15:09
Copy link
Author

@blabno blabno left a comment

Choose a reason for hiding this comment

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

Just a comment

textArea.setPrefSize(800, 600);

Scene scene = new Scene(textArea);
Stage stage = new Stage();
Copy link
Author

Choose a reason for hiding this comment

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

Most of this code comes from ContractWindow.java.
That class also sets owner (I'm not sure what that is for).
https://github.com/bisq-network/exchange/blob/master/gui/src/main/java/io/bisq/gui/main/overlays/windows/ContractWindow.java#L205

@ripcurlx ripcurlx added the in:gui label Feb 6, 2018
@cbeams cbeams requested review from cbeams and removed request for ManfredKarrer February 8, 2018 07:27
Copy link
Contributor

@cbeams cbeams left a comment

Choose a reason for hiding this comment

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

ACK.

I've tested these changes and they work well. @keo-bisq, heads-up: you can now double-click on arbitration chat messages and a window like the one below will pop up, where you can select and copy any portion of the text.

@bisq-network/docs-maintainers, this is also good for users to know, and is something we should add to our (currently non-existent) documentation.

image

Thanks, @blabno!

@cbeams cbeams merged commit 6499cf8 into bisq-network:master Feb 8, 2018
cbeams added a commit that referenced this pull request Feb 8, 2018
…at-messages-selectable

Make arbitration chat messages selectable
@cbeams cbeams added this to the v0.6.6 milestone Feb 8, 2018
@cbeams
Copy link
Contributor

cbeams commented Feb 8, 2018

@bisq-network/docs-maintainers, this is also good for users to know, and is something we should add to our (currently non-existent) documentation.

Actually, we shouldn't rely on documentation to reveal these sort of "hidden features". We should add an affordance to the UI that makes it obvious to the user that this feature is there.

We could (1) add a second icon next to the "copy to clipboard" icon that's already there, or (2) just change the behavior of the existing button to open this dialog instead. I'm open to either. Thoughts, @ripcurlx, @ManfredKarrer, @keo-bisq?

@blabno, feel free to open a follow-on issue about this.

@ManfredKarrer
Copy link
Contributor

I would prefer to make the text in the chat selectable. That would not require any user instruction. I will not have time to look into it but it was not trivial as far I remember from the text component which is used. But with a bit of effort I am sure it can be made.

@ManfredKarrer
Copy link
Contributor

I had another look....
Seems there is no ready made component out there which does what we want.
Label does not support text selection. Textfield is not multiline and Textarea does not adjust to size as we need it.

The hacks i found are using an invisible Label for layout and put a Textarea with the correct size over it (taken from the label) and use styles to look like a label (transparent background).
here are some examples:
https://community.oracle.com/thread/2319231
https://stackoverflow.com/questions/22534067/copiable-label-textfield-labeledtext-in-javafx
https://stackoverflow.com/questions/46869986/javafx-selectable-label-textarea-without-padding-or-border?rq=1

Would be good to make a component which integrates that hack or even better merge the functionalities of Label and Textarea, thought that might be a bit more tricky I assume.

@ManfredKarrer
Copy link
Contributor

@ripcurlx Maybe you have an idea?

@ripcurlx
Copy link
Contributor

Maybe we could do something like this https://stackoverflow.com/questions/44173811/how-to-make-a-javafx-label-selectable on single mouse click. Besides that I also didn't find a perfect solution that is ready to use.

@ManfredKarrer
Copy link
Contributor

Did not like that so much to add a new behavior. It should just behave like expected to select a text.

@cbeams
Copy link
Contributor

cbeams commented Feb 20, 2018

@ManfredKarrer, do you want to revert this change after all?

@ManfredKarrer
Copy link
Contributor

I dont mind. It might be an improvement to current copy icon and as its for arbitrators only i dont see a big usability issue. A proper selectable label component seems to be not trivial and for now maybe not worth the effort.

@ManfredKarrer ManfredKarrer removed this from the v0.6.6 milestone Feb 23, 2018
@blabno blabno deleted the feature/1058-make-arbitration-chat-messages-selectable branch April 6, 2018 09:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants