[jira] [Commented] (TRINIDAD-2120) Use jQuery ThemeRoller skins with Trinidad
[ https://issues.apache.org/jira/browse/TRINIDAD-2120?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13452110#comment-13452110 ] Prakash Udupa commented on TRINIDAD-2120: - Do you have plans to address the review comments and check-in this code ?. I just wanted to make sure this does not fall through the cracks after having invested time. > Use jQuery ThemeRoller skins with Trinidad > -- > > Key: TRINIDAD-2120 > URL: https://issues.apache.org/jira/browse/TRINIDAD-2120 > Project: MyFaces Trinidad > Issue Type: Improvement > Components: Skinning >Affects Versions: 2.0.0-core >Reporter: Leonardo Uribe >Assignee: Leonardo Uribe > Attachments: cupertino+casablanca.png, redmond+casablanca-2.png, > redmond+casablanca.png, screenshot-trinidad-cupertino.PNG, > screenshot-trinidad-smoothness.PNG, screenshot-trinidad-sunny.PNG, > south-street+casablanca-2.png, TRINIDAD-2120-1.patch, TRINIDAD-2120-2.patch, > TRINIDAD-2120-4-changes-trinidad-impl-only.patch > > > Here is the original mail from Trasca Virgil: > http://markmail.org/search/?q=themeroller%20trinidad#query:themeroller%20trinidad+page:1+mid:byczdawpyj33zqoy+state:results > Mon, 25 Oct 2010 07:01:25 -0700 > Hi > > I am interested to get better skinning support in Apache MyFaces. I want > to > get MyFaces closely integrated with http://jqueryui.com/themeroller/ - I am > targeting MyFaces JSF1.2 branch. > > The end result should be the same with what PrimeFaces already did > - http://www.primefaces.org/themes.html > > My initial idea is to implemented a JQueryCssToMyFacesCss kind of compiler > which > will get as input the jquery CSS syntax and will dump MyFaces CSS syntax. > > I have few questions related with this: > * Did anybody tried something similar in the past - in the MyFaces > community? > * Do you think the approach is achievable? Do you have a better > suggestion? Is > the UI MyFaces CSS syntax a generic enough UI css framework or is making > MyFaces > specific assumptions? > * Is this doable only by implementing the previous compiler or the > MyFaces/Trinidad components should be touched also? > > Here is the documentation for jQuery UI CSS framework > > http://docs.jquery.com/UI/Theming/API > > Thank you, > Virgil > Investigating more about this possible improvement, I notice that jquery > themeroller themes does not require jquery to work. So what can we do? > We can take themeroller themes and generate a skin from trinidad. Trinidad > already has all the pieces of the pluzze (css parser/merger and a cool > skinning api) so we should just use it. > I tried to create a skin in this way: > > sunny.desktop > sunny > org.apache.myfaces.trinidad.desktop > > skins/themeroller/sunny/jquery-ui-1.8.14.custom.css > > > sunny.desktop > > skins/themeroller/trinidad-theme.css > > The first stylesheet is the reference to a generated jquery theme and the > addition is the file that does the integration with trinidad. So, ThemeRoller > generates the .css + image files and we just need to provide a reusable .css > file to reuse the css classes. In practice with just one file we can create > 20 or 30 trinidad themes in one move! > Obviously these skins are no match for casablanca theme, and will possibly > have some flaws (the same for any themeroller skin, right?), but I think it > is worth to try it. I'll attach some files here to show how it looks like. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (TRINIDAD-2120) Use jQuery ThemeRoller skins with Trinidad
[ https://issues.apache.org/jira/browse/TRINIDAD-2120?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13416077#comment-13416077 ] Walter Mourão commented on TRINIDAD-2120: - Hi Folks, Great work! I added the patches to Trinidad 2.1.0-SNAPSHOT (small changes in the pom.xml) and I was able to run the showcase. I did some small testing with JQuery UI and I've put the widgets working (see it at: http://youtu.be/Eww607FdOW4 ). I don't know why, but I could not use the $() function. I had to use the document.getElementById. I think it is a small issue. If somebody has a clue, please let me know, I will investigate further, later. I could not be confident if the patches will go to Trinidad trunk soon. What do you think ? > Use jQuery ThemeRoller skins with Trinidad > -- > > Key: TRINIDAD-2120 > URL: https://issues.apache.org/jira/browse/TRINIDAD-2120 > Project: MyFaces Trinidad > Issue Type: Improvement > Components: Skinning >Affects Versions: 2.0.0-core >Reporter: Leonardo Uribe >Assignee: Leonardo Uribe > Attachments: TRINIDAD-2120-1.patch, TRINIDAD-2120-2.patch, > TRINIDAD-2120-4-changes-trinidad-impl-only.patch, cupertino+casablanca.png, > redmond+casablanca-2.png, redmond+casablanca.png, > screenshot-trinidad-cupertino.PNG, screenshot-trinidad-smoothness.PNG, > screenshot-trinidad-sunny.PNG, south-street+casablanca-2.png > > > Here is the original mail from Trasca Virgil: > http://markmail.org/search/?q=themeroller%20trinidad#query:themeroller%20trinidad+page:1+mid:byczdawpyj33zqoy+state:results > Mon, 25 Oct 2010 07:01:25 -0700 > Hi > > I am interested to get better skinning support in Apache MyFaces. I want > to > get MyFaces closely integrated with http://jqueryui.com/themeroller/ - I am > targeting MyFaces JSF1.2 branch. > > The end result should be the same with what PrimeFaces already did > - http://www.primefaces.org/themes.html > > My initial idea is to implemented a JQueryCssToMyFacesCss kind of compiler > which > will get as input the jquery CSS syntax and will dump MyFaces CSS syntax. > > I have few questions related with this: > * Did anybody tried something similar in the past - in the MyFaces > community? > * Do you think the approach is achievable? Do you have a better > suggestion? Is > the UI MyFaces CSS syntax a generic enough UI css framework or is making > MyFaces > specific assumptions? > * Is this doable only by implementing the previous compiler or the > MyFaces/Trinidad components should be touched also? > > Here is the documentation for jQuery UI CSS framework > > http://docs.jquery.com/UI/Theming/API > > Thank you, > Virgil > Investigating more about this possible improvement, I notice that jquery > themeroller themes does not require jquery to work. So what can we do? > We can take themeroller themes and generate a skin from trinidad. Trinidad > already has all the pieces of the pluzze (css parser/merger and a cool > skinning api) so we should just use it. > I tried to create a skin in this way: > > sunny.desktop > sunny > org.apache.myfaces.trinidad.desktop > > skins/themeroller/sunny/jquery-ui-1.8.14.custom.css > > > sunny.desktop > > skins/themeroller/trinidad-theme.css > > The first stylesheet is the reference to a generated jquery theme and the > addition is the file that does the integration with trinidad. So, ThemeRoller > generates the .css + image files and we just need to provide a reusable .css > file to reuse the css classes. In practice with just one file we can create > 20 or 30 trinidad themes in one move! > Obviously these skins are no match for casablanca theme, and will possibly > have some flaws (the same for any themeroller skin, right?), but I think it > is worth to try it. I'll attach some files here to show how it looks like. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (TRINIDAD-2120) Use jQuery ThemeRoller skins with Trinidad
[
https://issues.apache.org/jira/browse/TRINIDAD-2120?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13406213#comment-13406213
]
Prakash Udupa commented on TRINIDAD-2120:
-
Reg. -ui-icon issue: In order to accomodate that thermoroller has selectors
ending in 'ui-icon' that aren't to be treated like icon, we'd not want to break
other apps that ended their selectors with 'ui-icon' because they wanted to be
treated like icon. I'm assuming that you are not making this change now...
- return (selectorName.endsWith("-icon") ||
+ return ( (selectorName.endsWith("-icon") && !selectorName.equals("ui-icon")
&& !selectorName.contains(".ui-icon")) ||
b.t.w. -tr-icon is a good idea, I'm not sure if it is a common requirement to
invest time to do that, maybe you can file a bug on JQuery to see if they can
change the selector names to not end in 'icon'.
Reg. isColor(), I understand one is supposed to parse and return a color and
you need a method that just says whether it is a color. But then your method is
doing all the parsing that the other method is doing. Duplicate code is a pain
on maintenance. I'm suggesting that you just call parseColor() inside of your
isColor(), and return true for a successful parse, and false for a failure
(inlcudes exception case that you would catch and return false).
> Use jQuery ThemeRoller skins with Trinidad
> --
>
> Key: TRINIDAD-2120
> URL: https://issues.apache.org/jira/browse/TRINIDAD-2120
> Project: MyFaces Trinidad
> Issue Type: Improvement
> Components: Skinning
>Affects Versions: 2.0.0-core
>Reporter: Leonardo Uribe
>Assignee: Leonardo Uribe
> Attachments: TRINIDAD-2120-1.patch, TRINIDAD-2120-2.patch,
> TRINIDAD-2120-4-changes-trinidad-impl-only.patch, cupertino+casablanca.png,
> redmond+casablanca-2.png, redmond+casablanca.png,
> screenshot-trinidad-cupertino.PNG, screenshot-trinidad-smoothness.PNG,
> screenshot-trinidad-sunny.PNG, south-street+casablanca-2.png
>
>
> Here is the original mail from Trasca Virgil:
> http://markmail.org/search/?q=themeroller%20trinidad#query:themeroller%20trinidad+page:1+mid:byczdawpyj33zqoy+state:results
> Mon, 25 Oct 2010 07:01:25 -0700
> Hi
>
> I am interested to get better skinning support in Apache MyFaces. I want
> to
> get MyFaces closely integrated with http://jqueryui.com/themeroller/ - I am
> targeting MyFaces JSF1.2 branch.
>
> The end result should be the same with what PrimeFaces already did
> - http://www.primefaces.org/themes.html
>
> My initial idea is to implemented a JQueryCssToMyFacesCss kind of compiler
> which
> will get as input the jquery CSS syntax and will dump MyFaces CSS syntax.
>
> I have few questions related with this:
> * Did anybody tried something similar in the past - in the MyFaces
> community?
> * Do you think the approach is achievable? Do you have a better
> suggestion? Is
> the UI MyFaces CSS syntax a generic enough UI css framework or is making
> MyFaces
> specific assumptions?
> * Is this doable only by implementing the previous compiler or the
> MyFaces/Trinidad components should be touched also?
>
> Here is the documentation for jQuery UI CSS framework
>
> http://docs.jquery.com/UI/Theming/API
>
> Thank you,
> Virgil
> Investigating more about this possible improvement, I notice that jquery
> themeroller themes does not require jquery to work. So what can we do?
> We can take themeroller themes and generate a skin from trinidad. Trinidad
> already has all the pieces of the pluzze (css parser/merger and a cool
> skinning api) so we should just use it.
> I tried to create a skin in this way:
>
> sunny.desktop
> sunny
> org.apache.myfaces.trinidad.desktop
>
> skins/themeroller/sunny/jquery-ui-1.8.14.custom.css
>
>
> sunny.desktop
>
> skins/themeroller/trinidad-theme.css
>
> The first stylesheet is the reference to a generated jquery theme and the
> addition is the file that does the integration with trinidad. So, ThemeRoller
> generates the .css + image files and we just need to provide a reusable .css
> file to reuse the css classes. In practice with just one file we can create
> 20 or 30 trinidad themes in one move!
> Obviously these skins are no match for casablanca theme, and will possibly
> have some flaws (the same for any themeroller skin, right?), but I think it
> is worth to try it. I'll attach some files here to show how it looks like.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/
[jira] [Commented] (TRINIDAD-2120) Use jQuery ThemeRoller skins with Trinidad
[
https://issues.apache.org/jira/browse/TRINIDAD-2120?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13405727#comment-13405727
]
Leonardo Uribe commented on TRINIDAD-2120:
--
Thanks for take your time to review it. The feedback is most welcome.
I agree with the suggestions done, just some comments:
ui-icon : the issue with ui-icon is themeroller uses a selector class called
.ui-icon, but in trinidad, selectors ending with -icon has an special meaning.
See this comment in StyleUtils:
// =-=jmw There is no good way to tell if this is an icon.
// for now, I look at the selector name.
// we do have some styles that have -icon- in the name, but it's
// not at the end which is how icons are determined.
// our icon names look like .AFWarningIcon:alias
// AFErrorIconStyle is a style.
// This supports pseudo-classes on icon definitions (e.g.,
// foo-icon:hover- or FooIcon:alias:hover)
// -icon: is a condition because it could be -icon:hover.
It could be good to have something like this:
.ui-icon {
-tr-icon: false;
}
Or something like that, to indicate the selector should not be deal as an icon.
It has sense, and it shouldn't be hard to implement. In that way, you can add
the required entries in the base skin to prevent these selectors to be read as
icons.
CSSUtils.isColor() : the problem with use parseColor is if the parsing cannot
be done, an exception is thrown, so you need to catch it again and return
false. In the extraction process, the detection of the color property is based
into a "mutually exclusive" condition, which means the first value that could
be parsed into a color wins. In CSS spec that's ok, because it defines the
possible values to be found. I know the code is almost the same, but one is
doing a check, the other one is used to get the color, so both are different
methods, even if they look the same.
> Use jQuery ThemeRoller skins with Trinidad
> --
>
> Key: TRINIDAD-2120
> URL: https://issues.apache.org/jira/browse/TRINIDAD-2120
> Project: MyFaces Trinidad
> Issue Type: Improvement
> Components: Skinning
>Affects Versions: 2.0.0-core
>Reporter: Leonardo Uribe
>Assignee: Leonardo Uribe
> Attachments: TRINIDAD-2120-1.patch, TRINIDAD-2120-2.patch,
> TRINIDAD-2120-4-changes-trinidad-impl-only.patch, cupertino+casablanca.png,
> redmond+casablanca-2.png, redmond+casablanca.png,
> screenshot-trinidad-cupertino.PNG, screenshot-trinidad-smoothness.PNG,
> screenshot-trinidad-sunny.PNG, south-street+casablanca-2.png
>
>
> Here is the original mail from Trasca Virgil:
> http://markmail.org/search/?q=themeroller%20trinidad#query:themeroller%20trinidad+page:1+mid:byczdawpyj33zqoy+state:results
> Mon, 25 Oct 2010 07:01:25 -0700
> Hi
>
> I am interested to get better skinning support in Apache MyFaces. I want
> to
> get MyFaces closely integrated with http://jqueryui.com/themeroller/ - I am
> targeting MyFaces JSF1.2 branch.
>
> The end result should be the same with what PrimeFaces already did
> - http://www.primefaces.org/themes.html
>
> My initial idea is to implemented a JQueryCssToMyFacesCss kind of compiler
> which
> will get as input the jquery CSS syntax and will dump MyFaces CSS syntax.
>
> I have few questions related with this:
> * Did anybody tried something similar in the past - in the MyFaces
> community?
> * Do you think the approach is achievable? Do you have a better
> suggestion? Is
> the UI MyFaces CSS syntax a generic enough UI css framework or is making
> MyFaces
> specific assumptions?
> * Is this doable only by implementing the previous compiler or the
> MyFaces/Trinidad components should be touched also?
>
> Here is the documentation for jQuery UI CSS framework
>
> http://docs.jquery.com/UI/Theming/API
>
> Thank you,
> Virgil
> Investigating more about this possible improvement, I notice that jquery
> themeroller themes does not require jquery to work. So what can we do?
> We can take themeroller themes and generate a skin from trinidad. Trinidad
> already has all the pieces of the pluzze (css parser/merger and a cool
> skinning api) so we should just use it.
> I tried to create a skin in this way:
>
> sunny.desktop
> sunny
> org.apache.myfaces.trinidad.desktop
>
> skins/themeroller/sunny/jquery-ui-1.8.14.custom.css
>
>
> sunny.desktop
>
> skins/themeroller/trinidad-theme.css
>
> The first stylesheet is the reference to a generated jquery theme and the
> addition is the file that does the integration with trinidad. So, ThemeRoller
> generates the .css + image files and we just need to provide a reusable .css
> file to reuse the css classes. In practice
[jira] [Commented] (TRINIDAD-2120) Use jQuery ThemeRoller skins with Trinidad
[
https://issues.apache.org/jira/browse/TRINIDAD-2120?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13405393#comment-13405393
]
Prakash Udupa commented on TRINIDAD-2120:
-
General comments:
=
Trinidad-2120 could have been used to track discussion to JQuery ThemeRoller
integration. For specific feature code changes, I suggest you report specific
JIRA issues. There is possibly two that I'm seeing from your patch...
1. Enhance -tr-property-ref to be able to include specific segments of property
values in a shorthand property
2. Looks like you are trying to solve another requirement / issue regarding
'ui-icon'.
Review comments for code change in
'TRINIDAD-2120-4-changes-trinidad-impl-only.patch':
===
I liked the 'strategy' pattern you used, that makes it easy to expand this
feature to other properties that can take shorthand values.
General Documentation:
http://myfaces.apache.org/trinidad/devguide/skinning.html
There is a section that describes the -tr-property-ref feature. Please add
notes about the enhancement you did here.
StyleUtils.java:
---
You are letting selectorNames ending with 'ui-icon' as not being icons.
-return (selectorName.endsWith("-icon") ||
+return ( (selectorName.endsWith("-icon") &&
!selectorName.equals("ui-icon") && !selectorName.contains(".ui-icon")) ||
There are no comments around this change on why this is required. From the dev
guide, the documented convention is that all selectors ending in 'icon' are to
be specially treated as icons (they result in icon objects). There could be
legit icons selectors whose names end with 'ui-icon', and those will possibly
break with your change.
IncludePropertyNode.java:
--
public IncludePropertyNode(
String name,
String selector,
String propertyName,
String localPropertyName,
String location)
- There is no javadoc on the 'location' property. Please include an example
when you doc it.
- And then this constructor has duplicate code. The other constructor's body
should be just calling your new one passing a 'null' location.
- location / getLocation is confusing, if I understand this right, it is
specifically the CSS standard name of the property segment inside of a
shorthand property that you are trying to extract. Can you consider giving it a
more intuitive name, for example:
segmentPropertyName / getSegmentPropertyName
SkinStyleSheetParserUtils.java:
--
private static IncludePropertyNode _createIncludePropertyNode(
String propertyName,
String propertyValue)
{
if (values.length == 3)
{
location = trimQuotes(values[2].trim());
}
...
}
- Remove white space line inside the if statement.
StyleSheetDocument.java:
--
You handled the use-case #2:
// 2. Resolve included properties
// color: -tr-property-ref(".AFDefaultFont:alias","background-color");
- Looks like you missed to handle the use-case for #2-prime:
// 2'. Resolve included properties in a compact selector.
// e.g., border: 1px solid -tr-property-ref(".AFDarkColor:alias",color);
I see #2-prime is as much a common usecase as is #2
- Make "CSSUtils._isLength(String value)" as a public method, remove the
underscore prefix, and then just use it in LengthExtractor
- Add a similar isBorderStyle() method to CSSUtils and use it in
BorderStyleExtractor.
- You added a public static method trimQuotes() that is a duplicate of the
already existing _trimQuotes(), remove the public method.
- Consider renaming 'COMPOSITE_SELECTORS' to
'COMPOSITE_PROPERTIES_EXTRACTOR_MAP' because that is what it is, consider
adding a code comment briefly explaining this data structure.
- For sake of improved readability, move all the private static extractor
classes towards the end of the file where all the private static classes are,
and also move the static initializer block to after all the static classes.
- Add javadoc style comments to Extractor interface .
// used when resolving -tr-property-ref
// given a list of PropertyNodes to traverse, find the one whose name matches
the propertyName.
// then we assign the value to the localPropertyName.
// .AFFoo:alias {color: red} af|test {background-color:
-tr-property-ref(".AFFoo:alias", "color");
// localPropertyName is background-color
// propertyName is color
// properties is .AFFoo:alias's properties
private List _getIncludedProperties(
String propertyName,
String localPropertyName,
Iterable properties)
{
- This method is no more needed because you took care of its implementation in
the new method you added that takes the additional 'location' parameter. Please
remove this method, and make
