[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-openjfx-dev
Subject:    API REVIEW: move com.sun.javafx.css.StyleManager to javafx.scene.StyleManager
From:       david.grieve () oracle ! com (David Grieve)
Date:       2012-07-25 21:03:18
Message-ID: C6A613DE-DE9D-497D-A6B5-33E35E9B6759 () oracle ! com
[Download RAW message or body]

1. There is no impact on any future API to support programmatic CSS access \
(http://javafx-jira.kenai.com/browse/RT-17293). 

2. StyleManager should be a final variable in Scene. Even if more of the css \
implementation were to be moved to javafx.scene (something Richard is arguing for), \
the getter would still need to be public since StyleManager is used outside of \
javafx.scene (in javafx.scene.controls and possibly by SceneBuilder)

3. I didn't notice the cut-n-paste error. The two methods should be \
setDefaultUserAgentStylesheet and addUserAgentStylesheet. Both are not strictly \
required. addUserAgentStylesheet could be enough. However, normal declarations from \
the default user agent stylesheet need be first in the cascade so they may be \
overwritten by declarations from other user agent stylesheets. Assuming the first UA \
stylesheet is meant to be the default is not a valid assumption. The list of UA \
stylesheet URL strings could be exposed, but then anyone could come along and insert \
their UA stylesheet and make it the default. Maybe that isn't a bad thing, but it is \
a change in how the system behaves.

The two methods allow UA stylesheets to be added even before the default is set. It \
isn't fool proof. There would be nothing to prevent someone from calling setDefault \
UserAgentStylesheet before controls calls it to set caspian. Again, maybe this isn't \
a bad thing. 

On Jul 25, 2012, at 4:05 PM, Jonathan Giles wrote:

> Personally I have no issue with doing this (StyleManager on Scene and Parent), but \
> then I do not own these two classes. I do have a few questions though: 
> 1) How does this impact any future APIs to support programmatic CSS access (i.e. \
> Java API to get/set/observe CSS property values)? 
> 2) In general we try not to have getters/setters when the object is not being \
> exposed as a full property - but I guess it makes no sense to allow people to set \
> or observe changes to the StyleManager field? 
> 3) I know I could dive into the code, but what is the difference between \
> setDefaultUserAgentStylesheet and addDefaultUserAgentStylesheet? Are both \
> necessary? 
> -- Jonathan
> 
> On 26/07/2012 5:15 a.m., David Grieve wrote:
> > Background:
> > This request is related to http://javafx-jira.kenai.com/browse/RT-23460. \
> > StyleManager's job it is to keep track of stylesheets, manage the cascade, and \
> > create StyleHelpers for nodes. StyleHelper is the code that applies css styles to \
> > nodes. 
> > Currently, there StyleManager is a singleton but it can manage stylesheets from \
> > many different scenes. To do this, it maintains a table that maps Scene to a \
> > StylesheetContainer which contains the list of declarations from all the \
> > stylesheets associated with the scene. RT-23460 seeks to eliminate the lookup of \
> > Scene to StylesheetContainer. One such way to do this is for Scene to own its \
> > StylesheetContainer, as is proposed in the bug report. Another way is to have \
> > Scene own its own StyleManager. This latter approach is preferred (well, by me at \
> > least) since it greatly reduces the complexity of StyleManager and makes other \
> > optimizations possible. 
> > Proposed changes:
> > 
> > I propose adding a StyleManager member to Scene and moving StyleManager to the \
> > javafx.scene package as an abstract class with the minimum required public API. 
> > Scene would add:
> > private final StyleManger styleManager;
> > public final StyleManager getStyleManager()
> > 
> > Because Scene would have public API that returns a StyleManager, StyleManager \
> > itself needs to be public API and would need to be moved from com.sun.javafx.css \
> > to javafx.scene I propose StyleManager's public API as:
> > public abstract StyleHelper getStyleHelper(Node node)
> > public final ObservableList<String> getStylesheets() -- the list of stylesheet \
> > URL strings for the Scene. Scene's getStylesheets() method would simply return \
> > styleManager.getStylesheets() public void setDefaultUserAgentStylesheet(String \
> > url) public final void addDefaultUserAgentStylesheet(String url) -- support \
> > Controls that have their own UA stylesheet, plus adding UA stylesheets for \
> > embedded public final List<String> getUserAgentStylesheets() - return immutable \
> > list of UA stylesheet URL strings Note that StyleManager returns a StyleHelper. \
> > Hence, StyleHelper would also need to be part of the public API. There are only \
> > two methods in StyleManager that need to be exposed public abstract void \
> > transitionToState() - this method is called from Node when css needs to be \
> > applied public abstract boolean isPseudoclassUsed(String pseudoclass) - this \
> > method is also called from node to determine whether or not the node's \
> > pseudo-class state would cause css to be applied 
> > Note that neither the proposed StyleManager API nor the StyleHelper API pull in \
> > any other API not already part of the public API. 
> > Having Scene own a StyleManager also has the benefit of being able to avoid \
> > looking up the parent chain to see if the scene's window is a popup or not. That \
> > logic can be moved to the constructor. With this change, StyleManager can be \
> > further streamlined by eliminating the StylesheetContainer (StylesheetContainer \
> > gets moved into the body of the StyleManagerImpl) and the defaultContainer. 
> > Further, since a Parent can also have stylesheets, I also propose adding \
> > StyleManager to Parent with the same API as in Scene. private final StyleManger \
> > styleManager; public final StyleManager getStyleManager()
> > 
> > This will help eliminate the parent stylesheet lookup that currently happens in \
> > StylesheetContainer. 
> > 
> > David Grieve | Principal Member of Technical Staff
> > Mobile: +16033121013
> > Oracle Java Client UI and Tools
> > Durham, NH 03824
> > Oracle is committed to developing practices and products that help protect the \
> > environment 
> 


David Grieve | Principal Member of Technical Staff
Mobile: +16033121013 
Oracle Java Client UI and Tools
Durham, NH 03824 
 Oracle is committed to developing practices and products that help protect the \
environment


[prev in list] [next in list] [prev in thread] [next in thread]