Adjusting applications using the old weblaf to work with new version

[daWoife]

The styling technics in the 'new' weblaf are really a big improvement, good work you have done there the last years. I'm trying to adjust an application using the 'old' weblaf for years now to work with the new weblaf. The most work is done, it was not too difficult to replace the old UI calls. One problem I had was that I can not use the StyleManager directly to set the styleId of a component because our application uses the UIManager for setting the look and feel, there are no imports of weblaf packages in our code. But the styleId for a component can also be set with it's putClientProperty - method, so this was a good way to do it. But currently there is no way to set a state for a component without implementing the Stateful interface. And that is a problem if you can not import weblaf packages. So there should be also a way to set the states with the putClientProperty - method. To solve this problem I changed the static method DecorationUtils.getExtraStates to return additional states someone can define in a comma separated string property for the object parameter of the method. I therefore used the existing property 'decorationStates' from AbstractDecorationPainter. I'm not sure but I think this property is currently only used as a trigger and the value of the property is not used. What do you think about the problem and my solution, and do you know a better way ?

[mgarin]

This is actually a really good idea! The only thing I would add - decoration painter will have to listen to that property change, otherwise it won't be properly re-read on change.

Also, thinking about this feature in general - it is useful not only for the case you've described, but also in case you simply don't want to implement Stateful for any particular component, but want to provide custom states for your style. Web* components can also have a public API for this so you won't need to interact with the client properties.

I'll look into this on the next week and probably commit the implementation so it will be included in the next update. It is a rather small feature, so there should be no issues.

[daWoife]

I thought just using the 'decorationStates' property will do the trick because AbstractDecorationPainter is already listening to that property and calls updateDecorationStates (and so DecortationUtils.getExtraStates, revalidate and repaint) for a component if putClientProperty is used. But I will see your implementation and what's missing on mine.

I think another good idea would be to make no restrictions on the class of the object someone uses as property value for 'decorationStates'. Not only a comma separated string should be allowed, also a list, array or any user object. The use of the toString method of the object can do all that.

if ( object instanceof JComponent )
{
    final Object extra = ( ( JComponent ) object ).getClientProperty( AbstractDecorationPainter.DECORATION_STATES_PROPERTY );
    if ( extra != null )
    {
        if ( extra instanceof Object[] )
        {
            extra = Arrays.toString ( ( Object[] ) extra );
        }
        states = TextUtils.stringToList ( extra.toString ( ), " \t,;[]" );
    }
}

[mgarin]

I thought just using the 'decorationStates' property will do the trick because AbstractDecorationPainter is already listening to that property and calls updateDecorationStates (and so DecortationUtils.getExtraStates, revalidate and repaint) for a component if putClientProperty is used. But I will see your implementation and what's missing on mine.

It does listen to it's change indeed, but I'm not yet sure if I'd rather have a separate properly for client properties or use that one. Currently 'decorationStates' is simply used to inform painter about states change but it isn't an actual property anywhere.

I think another good idea would be to make no restrictions on the class of the object someone uses as property value for 'decorationStates'. Not only a comma separated string should be allowed, also a list, array or any user object. The use of the toString method of the object can do all that.

I would rather prefer property to accept one strict format, converting states from array/list to string and then back to list is quite a waste of processing time, even if not much - it might stack up. Some component states get requested/updated pretty often. It doesn't matter on a scale of 1 component, but when your UI has hundreds of components it will become more prominent.

[mgarin]

As alternative - I might add some class acting as a convenient wrapper for the component states to use instead of ArrayList (or array), but I'm not yet sure if it will be worth the API breaks it can potentially bring.

[daWoife]

I also first thought about using an extra property to implement the custom property states, but then I realized that it is not necessary as long as the listeners do not use the oldValue and newValue fields of the PropertyChangeEvent and read the current value of the property via getClientProperty instead. So the static DecorationsUtil.fireStateChange method still can fire property change events with null values. But maybe having an extra property like 'customStates' or so is a better and 'cleaner' solution, you will see. You are right with the performance aspect of my solution, I didn't thought about that. So to restrict the allowed objects for the property to either String, List\<String> or String[] is also a reasonable implementation. A wrapper class has the same problem as the Stateful interface. If you can not import weblaf packages you can not use it. So I dont think such a class is useful or necessary.

[mgarin]

Just an update on this - I haven't forgotten about this improvement, but since 1.3.0 is taking a while to complete - I will probably release a few more minor updates (before 1.3.0 is released) where I will add this change.

[mgarin]

A wrapper class has the same problem as the Stateful interface. If you can not import weblaf packages you can not use it. So I dont think such a class is useful or necessary.

That is a good point, I guess List will probably be the best choice.

[daWoife]

That's true, List<String> is the best choice in general, so that a user easily can add / remove states, but maybe more often only one state will be given to a component. So a simple String with one state should also be possible to ease things for the user of the property.

[mgarin]

You are probably right about the most common case being 1 state, so I'll have that as an option.

[mgarin]

I've been trying to implement this feature and found that it conflicts with Stateful interface functionality in case I am providing some API to conveniently add/remove states in Web* components.

To be more specific - if I'm adding methods for supporting client property for custom states into Web* components - along with set/add/remove methods for states there should also be a method to retrieve current states as list (from client property), something like public List<String> getStates (), but this directly conflicts with Stateful interface method that has the same signature.

One solution is to avoid that method at all for client property, but then it gets extremely confusing if you also implement Stateful for the same component as they are not connected anyhow. It is also quite confusing overall that there are two completely different approaches to providing custom states. I am certainly in favor of client properties as it will be easier to use, but then something has to be done with Stateful interface.

So, the possible way to go is to merge Stateful and the property together. My main problem with Stateful is that you have to manually fire state change events right now and it is inconvenient - client property approach solves that problem and should be quite optimized as well if implemented correctly. Here is an example of current Stateful use case:

    /**
     * Custom combobox separator placed between renderer/editor and button.
     */
    public class ComboBoxSeparator extends WebSeparator implements Stateful, EditabilityListener, VisibilityListener
    {
        /**
         * Constructs new combobox separator.
         */
        public ComboBoxSeparator ()
        {
            super ( StyleId.comboboxSeparator.at ( comboBox ), HORIZONTAL );
        }

        /**
         * Installs custom combobox listeners.
         */
        public void install ()
        {
            addEditabilityListener ( this );
            addPopupVisibilityListener ( this );
        }

        /**
         * Uninstalls custom combobox listeners.
         */
        public void uninstall ()
        {
            removePopupVisibilityListener ( this );
            removeEditabilityListener ( this );
        }

        @Override
        public void visibilityChanged ( final boolean visible )
        {
            DecorationUtils.fireStatesChanged ( this );
        }

        @Override
        public void editabilityChanged ( final boolean editable )
        {
            DecorationUtils.fireStatesChanged ( this );
        }

        @Nullable
        @Override
        public List<String> getStates ()
        {
            final List<String> states = new ArrayList<String> ( 1 );
            if ( comboBox.isEditable () )
            {
                states.add ( DecorationState.editable );
            }
            states.add ( isPopupVisible ( comboBox ) ? DecorationState.expanded : DecorationState.collapsed );
            return states;
        }
    }

Let's say we are going to merge Stateful and client property and provide support in all Web* components including WebSeparator used in this code, the implementation would be:

    /**
     * Custom combobox separator placed between renderer/editor and button.
     */
    public class ComboBoxSeparator extends WebSeparator implements EditabilityListener, VisibilityListener
    {
        /**
         * Constructs new combobox separator.
         */
        public ComboBoxSeparator ()
        {
            super ( StyleId.comboboxSeparator.at ( comboBox ), HORIZONTAL );
        }

        /**
         * Installs custom combobox listeners.
         */
        public void install ()
        {
            addEditabilityListener ( this );
            addPopupVisibilityListener ( this );
        }

        /**
         * Uninstalls custom combobox listeners.
         */
        public void uninstall ()
        {
            removePopupVisibilityListener ( this );
            removeEditabilityListener ( this );
        }

        @Override
        public void visibilityChanged ( final boolean visible )
        {
            if ( isPopupVisible ( comboBox ) )
            {
                addState ( DecorationState.expanded );
            }
            else
            {
                removeState ( DecorationState.collapsed );
            }
        }

        @Override
        public void editabilityChanged ( final boolean editable )
        {
            if ( comboBox.isEditable () )
            {
                addState ( DecorationState.editable );
            }
            else
            {
                removeState ( DecorationState.editable );
            }
        }
    }

I can also add some conditional methods into API for convenience:

        @Override
        public void visibilityChanged ( final boolean visible )
        {
            switchState ( isPopupVisible ( comboBox ), DecorationState.expanded, DecorationState.collapsed );
        }

        @Override
        public void editabilityChanged ( final boolean editable )
        {
            switchState ( comboBox.isEditable (), DecorationState.editable );
        }

So basically instead of providing states in a single method you can freely add/remove them whenever necessary and underlying implementation will update decoration accordingly. This is especially good if you have a lot of custom states that are dependent on different conditions - with property approach each decoration update will not re-check all conditions, it will simply take states from property.

The main problem is - this might take a while to implement as I have quite a few Stateful uses across WebLaF and I'll need to adjust them all. Another problem - even though I doubt a lot of developers used Stateful, this is still an API break.

Either way, I'm still working on this feature and will see if it will make into v1.2.13 that will be released most probably this Friday.

[mgarin]

Also, to give you an idea of what API methods for the client property states I'm talking about - you can take a look at current WebCanvas implementation which currently uses Stateful: WebCanvas.java#L99

[daWoife]

Well, I'm not sure if it would be a good idea to add all these methods for add/remove states to all of the Web* components. It really would be a lot of work and its not necessary, isn't it ? Wouldn't it be enough to have all the methods from WebCanvas in DecorationUtils ? public void setStates ( final JComponent component, final String ... states ); public void addStates ( final JComponent component, final String ... states ); .. The switchState methods of your example can also be there. The methods in DecorationUtils would work with the client property and not with a class attribute like in WebCanvas. Then you can replace all your Stateful implementations with calls to DecorationUtils. If you have done that and Stateful isn't used any more in your code you can deside later if you want the remove the Stateful interface or still let it be there for compatibility reasons.

[mgarin]

I wouldn't say it's unnecessary - the general idea of all Web* components is to have more convenient and simple APIs to control component settings which in this case includes custom states. As the styling becomes more important - there should be a standard way to customize states without resorting to making a custom component on top of already existing ones.

And the point for having these APIs is exactly to avoid using various static utilities. While the actual method implementations will surely be in a separate utility class and will be used by each of the Web* components - it is never a good idea to promote their direct usage as it is neither convenient nor good for the resulting code. That being said - they will still be useable directly for J components support reasons.

P.S. Sorry for the delay with the answer, I've been unable to really work with anything for a week due to illness, getting back into action now :)

[daWoife]

No problem, hope you are well again. I have to admit that I didn't looked too much into the Web* components in the past because as you know I don't use them. So I'm sure you are right with your explanation why the state methods should be in there. And if there will be a separate utility class for the use with J components I can also use it with the reflection api. This would be quite convenient because then I don't have to bring this funtionality in our code. :-)

[mgarin]

Utility is mostly there for internal use or J-components. In case you don't want any dependencies - you will still be able to simply provide states directly into client property (and it should still be more convenient compared to using Reflection) - that part isn't going anywhere, I'll just be adding some convenience on top of it in Web* components and updating some APIs to work properly with it.

[mgarin]

I'll be adding this with v1.2.14 update not to delay v1.2.13 further (which will soon be going live).