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

Panel's foreground is inherited to all contained styled labels #562

Closed
mgarin opened this issue Oct 14, 2019 · 2 comments
Closed

Panel's foreground is inherited to all contained styled labels #562

mgarin opened this issue Oct 14, 2019 · 2 comments
Assignees

Comments

@mgarin
Copy link
Owner

mgarin commented Oct 14, 2019

@Sciss mentioned on our discussion on Gitter that whenever panel's <foreground> setting is specified - all styled labels (and some other text components) inherit that foreground color.

I already did some investigation and there are multiple reasons that cause such behavior:

  1. AbstractTextContent only uses it's own color from the style if there is no non-UIResource color specified in component's foreground
  2. WebStyledLabel actually doesn't have it's own foreground, it's basically null for it
  3. All colors deserialized from the style right now are NOT ColorUIResources but simple Colors
  4. All Swing components actually inherit multiple settings from parent, including foreground, background and some others, here is a foreground code piece from Component:
    public Color getForeground() {
        Color foreground = this.foreground;
        if (foreground != null) {
            return foreground;
        }
        Container parent = this.parent;
        return (parent != null) ? parent.getForeground() : null;
    }

All these reasons together cause the mentioned behavior.

There are a few things that have to be done to fix this and avoid such issues in the future:

  1. Ensure that all Colors read from style are read as ColorUIResources
  2. Probably specify default foregrounds for text components in style instead of the L&F as it is to an unreliable source and also doesn't account for the current skin
  3. Adjust text component styles accordingly
@mgarin mgarin added this to the v1.2.11 milestone Oct 14, 2019
@mgarin mgarin self-assigned this Oct 14, 2019
@mgarin
Copy link
Owner Author

mgarin commented Oct 16, 2019

There were a few more issues on top of mentioned above that I've found while trying to fix this.

One of the issues is that disabled colors from style would not be prioritized anymore if user would set, let's say, a JLabel foreground - that foreground will persist for disabled state as well because it is not a UIResource and the current text content implementation always favors non-UIResource options to avoid having user settings overwritten by the style ones. I will fix this by adding a separate setting to ignore user color for particular states - for instance for disabled states.

There is also an issue with WebDocumentPane tab colors being inherited from the parent despite the fix I've made for it. It happened because it overwritten foreground with null value and forced tab title to inherit color once again.

And there is also an issue with WebTabbedPane tab titles not respecting colors specified through setForegroundAt ( int, Color ) - this will get patched by custom text contents for the Tab element in the style (it is basically a styledlabel used as tab title).

I'm wrapping up with the last fixes right now and hopefully will soon push the fix.

mgarin added a commit that referenced this issue Oct 16, 2019
- ConverterContext.java - New context for passing `ThreadLocal` settings across different XStream converters
- ComponentStyleConverter.java - Added `ConverterContext` usage to ensure all subsequent resources read as `UIResource`s when possible
- ColorUIResource.java - Added for internal WebLaF usage as `UIResource` for `Color` settings
- ColorConverter.java, InsetsConverter.java - Adjusted to account for `ConverterContext` settings
- AbstractTextContent.java - Added `ignoreCustomColor` property to allow disabling custom color prioritization in style
- AcceleratorText.java, LocaleTextContent.java - These text contents will now prioritize their own color over custom component's
- Adjusted all text component styles to ensure that states like `disabled` preserve their custom text color
- Added `foreground` property for all base components to ensure it is available and correct
- Adjusted all component styles to ensure they have correct foreground values for different skins

Menus
- StyleId.java, menu.xml, menuitem.xml, checkboxmenuitem.xml, radiobuttonmenuitem.xml - Added base `styled` style for styled text support

TabbedPane
- TabText.java, TabText.xsd - New custom text content for tabs that uses proper user color fallback to ensure it is accounted for
- StyleManager.java - Added annotation processing for `TabText`
- tabbedpane.xml - Adjusted to use `TabText` instead of `StyledLabelText`

DocumentPane
- TabTitleComponent.java - Custom foreground is now only set if not `null` to avoid affecting foreground usage behavior

OptionPane
- optionpane.xml - Fixed inconsistency between light and dark skins

SyntaxArea
- WebSyntaxArea.java - Temporary changed default theme initialization and update to adopt to basic skins
- WebSyntaxArea.java, SyntaxPreset.java, SyntaxTheme.java - Added `@NotNull` and `@Nullable` annotations

Utilities
- DocumentEventMethods.java, DocumentEventMethodsImpl.java - Added `@NotNull` annotations

DemoApplication
- Code and style source areas will now appear dark on the dark skin
@mgarin
Copy link
Owner Author

mgarin commented Oct 16, 2019

I've fixed the main issue and all of the related ones.
Hopefully there should be no more weird cases of color inheritance anywhere by default.

@mgarin mgarin closed this as completed Oct 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant