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

Label picker: unmatched labels look like disabled #1333 #1350

Merged
merged 2 commits into from
Apr 10, 2016

Conversation

tsoonjin
Copy link
Contributor

Fixes #1333

@tsoonjin tsoonjin force-pushed the 1333-fix-unmatched-label-display branch 2 times, most recently from 1d7635a to 1e20f5b Compare March 16, 2016 00:05
@tsoonjin tsoonjin force-pushed the 1333-fix-unmatched-label-display branch from 1e20f5b to ab54c53 Compare April 2, 2016 03:32
@damithc
Copy link
Contributor

damithc commented Apr 2, 2016

It's better. But as long as we use opacity, they are going to look 'disabled'. I think the solution is to make the label clickable when the mouse is over it. e.g. change mouse pointer and/or remove opacity

@tsoonjin
Copy link
Contributor Author

tsoonjin commented Apr 2, 2016

I think we just need a better way of highlighting the matched label from the rest e.g making it pop-up using size. In hindsight, using border is not really a good choice. What do you think ?

@damithc
Copy link
Contributor

damithc commented Apr 2, 2016

yes, it's nice if we can emphasize matching labels without de-emphasizing unmatched labels. Not sure how hard it is to do so. I don't want to do anything (such as increasing size) that changes the positioning of labels as the user types. In spite of whatever we do there, the labels need to look clickable when hovered over.

@tsoonjin
Copy link
Contributor Author

tsoonjin commented Apr 2, 2016

Ok. So as long as the position of the labels remain the same you are ok with it right ? Coupled that by changing the mouse pointer, I think it will be perfect already.

@damithc
Copy link
Contributor

damithc commented Apr 2, 2016

Yup. Let us know when the code is ready for review.

@tsoonjin
Copy link
Contributor Author

tsoonjin commented Apr 9, 2016

Here is the updated version where the cursor icon will change when hover on a clickable label.

unmatched_label

Ready for review @ndt93

// add highlight border
String style = getStyle() + (isHighlighted ? " -fx-border-color: black; -fx-border-width: 2px;" : "");
// change opacity if needed
style += (isFaded ? " -fx-opacity: 40%;" : "");
label.setStyle(style);
Copy link
Contributor

Choose a reason for hiding this comment

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

This style part can be extracted into another method.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

noted

@tsoonjin tsoonjin force-pushed the 1333-fix-unmatched-label-display branch 2 times, most recently from 3eadf31 to c75e913 Compare April 10, 2016 00:27
@tsoonjin tsoonjin force-pushed the 1333-fix-unmatched-label-display branch from c75e913 to 63abae6 Compare April 10, 2016 06:18
@tsoonjin tsoonjin changed the title Fix unmatched label UI #1333 Label picker: unmatched labels look like disabled #1333 Apr 10, 2016
@tsoonjin tsoonjin merged commit 63abae6 into HubTurbo:master Apr 10, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants