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

[Theming] Minor errors with the color chooser UI #390

Closed
ed-erwin-tf opened this issue Jun 28, 2022 · 7 comments
Closed

[Theming] Minor errors with the color chooser UI #390

ed-erwin-tf opened this issue Jun 28, 2022 · 7 comments
Assignees
Labels
6.0 - Iridium Release 6.0 - 2022.H2

Comments

@ed-erwin-tf
Copy link

There are several minor errors with the color chooser UI. I'll list them all here but can create separate issues if you prefer.

  1. The "Reset" button does not work as expected. It will change the color of the swatch at top, but does not reset the sliders or other items.

  2. Colored sliders on the ColorWheelChooser and the ColorSliderChooser have strange borders. Could they be related to miscalculation for fractional scaling? (I'm using Windows 250%) ColorWheelChooser

  3. In the ColorSliderChooser, the colors of the sliders do not always update when dragging other sliders. In one example picture, the Hue slider is red even though the chosen hue 81 is actually green. If you set Saturation to 0%, then when you change the hue slider, the color of the Saturation slider doesn't update. HSB Sliders
    HSB Sliders 2

  4. The Xoetrope color chooser has poor use of layout. Some text is tiny. Layout doesn't change when the dialog size changes. Some translated versions have text too long to fit in the given space.

french layout

I personally only care about #1 (and care a little about #2) because I turn off other choosers using
UIManager.put("ColorChooser.defaultChoosers",
new String[] {ColorWheelChooser.class.getName(), SwatchesChooser.class.getName()});

I do that because I need to translate my program into 15 languages and don't want to translate all of that text. (Some is already translated but not everything in every language I need.)

I realize this is externally contributed code, but maybe a fix could be done anyway.

Environment:
Radiance 5.0 (and earlier versions. I think the "Reset" button worked better in very old versions.)
Theming
Java 11 (Corretto) Windows 10, High DPI monitor with scaling at 250%

@kirill-grouchnikov
Copy link
Owner

Thanks for the detailed notes. I'll need to finish first with converting the components (aka Flamingo) to the new direct rendering world, so it might take a couple more weeks until I can start looking into this.

Can't promise much about the Xoetrope layout, as it's all hardcoded pixel-based values. Might be nice to consider how it can expand to more space, of course.

@kirill-grouchnikov kirill-grouchnikov self-assigned this Jun 29, 2022
@kirill-grouchnikov kirill-grouchnikov added the 6.0 - Iridium Release 6.0 - 2022.H2 label Jun 29, 2022
@ed-erwin-tf
Copy link
Author

Thanks. I don't really care about Xoetrope as I don't think my users would need that. A functional Reset button in the ColorWheelChooser is all I really need.

@kirill-grouchnikov kirill-grouchnikov changed the title Minor errors with the color chooser UI [Theming] Minor errors with the color chooser UI Jul 8, 2022
kirill-grouchnikov added a commit that referenced this issue Jul 13, 2022
* Move to 1x aware rendering for better visuals on high density displays
* Use skin colors for border and outside fill
* Use skin colors for ticks and draw them in one pass
* Fix alignment of slider thumb

For #390
kirill-grouchnikov added a commit that referenced this issue Jul 14, 2022
* Move to 1x aware rendering for better visuals on high density displays
* Use skin colors for borders

For #390
@kirill-grouchnikov
Copy link
Owner

Not sure about the "Reset" button yet. If I reset everything in every panel, that is quite destructive, losing all the changes in all the components in there. On the other hand, I do see the disconnect from the consistency perspective.

@kirill-grouchnikov
Copy link
Owner

"Reset" should be wired to reset all the panels now

Adaptive layout for the Xoetrope panel is a nice to have, but a rather low priority. Closing this as the rest of the comments should be addressed with the latest changes that went into 6.0-SNAPSHOT

@ed-erwin-tf
Copy link
Author

Thank you.

I need to localize the "OK", "Cancel", "Reset" buttons to a few languages you don't support. I haven't been able to figure out which java or properties files those values are coming from. Can you find them?

I would be happy to contribute the localized text for those buttons (though not for everything in your code.)

@ed-erwin-tf
Copy link
Author

Never mind. I have located these constants in Swing:
ColorChooser.okText
ColorChooser.cancelText
ColorChooser.resetText

@kirill-grouchnikov
Copy link
Owner

Yes, you can see them here - https://github.com/openjdk/jdk/blob/master/src/java.desktop/share/classes/javax/swing/JColorChooser.java#L682

Unfortunately that piece of code also means that I can't "take over" the layout of the button bar. I'd like to create some separation and put the reset button a bit further away from the other two buttons, as it's a destructive operation. But I don't see a way to do that in a custom look-and-feel.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.0 - Iridium Release 6.0 - 2022.H2
Projects
None yet
Development

No branches or pull requests

2 participants