[GitHub] struts pull request: WW-4634 Centre alignment doesn't seem to work...

2016-05-26 Thread lukaszlenart
Github user lukaszlenart commented on the pull request: https://github.com/apache/struts/pull/95#issuecomment-221785224 I assume we're ok with this and this PR can be merged? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as

[GitHub] struts pull request: WW-4634 Centre alignment doesn't seem to work...

2016-05-25 Thread lukaszlenart
Github user lukaszlenart commented on the pull request: https://github.com/apache/struts/pull/95#issuecomment-221621914 @victorsosa osm! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this

[GitHub] struts pull request: WW-4634 Centre alignment doesn't seem to work...

2016-05-25 Thread victorsosa
Github user victorsosa commented on the pull request: https://github.com/apache/struts/pull/95#issuecomment-221567053 I will fix the 103 test cases, give me some time --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If

[GitHub] struts pull request: WW-4634 Centre alignment doesn't seem to work...

2016-05-25 Thread lukaszlenart
Github user lukaszlenart commented on the pull request: https://github.com/apache/struts/pull/95#issuecomment-221556310 103 tests need to be updated, I think it isn't worth --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well.

[GitHub] struts pull request: WW-4634 Centre alignment doesn't seem to work...

2016-05-25 Thread lukaszlenart
Github user lukaszlenart commented on the pull request: https://github.com/apache/struts/pull/95#issuecomment-221556014 Tests are failing :( ``` Error Message

[GitHub] struts pull request: WW-4634 Centre alignment doesn't seem to work...

2016-05-25 Thread cnenning
Github user cnenning commented on the pull request: https://github.com/apache/struts/pull/95#issuecomment-221490265 :+1: --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled

[GitHub] struts pull request: WW-4634 Centre alignment doesn't seem to work...

2016-05-24 Thread aleksandr-m
Github user aleksandr-m commented on the pull request: https://github.com/apache/struts/pull/95#issuecomment-221326208 @victorsosa IMO it would be great. BUT if we really really want it to be backward compatible then we can do both. Add a class for future **and** use align attribute

[GitHub] struts pull request: WW-4634 Centre alignment doesn't seem to work...

2016-05-24 Thread victorsosa
Github user victorsosa commented on the pull request: https://github.com/apache/struts/pull/95#issuecomment-221314653 @aleksandr-m You mean something like in controlheader.ftl and .tdInput {text-align:left;} in styles.css. <-- This is for 2.5. THat means **drop align

[GitHub] struts pull request: WW-4634 Centre alignment doesn't seem to work...

2016-05-23 Thread aleksandr-m
Github user aleksandr-m commented on the pull request: https://github.com/apache/struts/pull/95#issuecomment-221056549 I would prefer to follow html5 and drop align attribute completely from S2 tags, BUT @victorsosa can you at least add some css class to input cell along with your

[GitHub] struts pull request: WW-4634 Centre alignment doesn't seem to work...

2016-05-21 Thread aleksandr-m
Github user aleksandr-m commented on the pull request: https://github.com/apache/struts/pull/95#issuecomment-220795005 @victorsosa You see, this commit already *broke* backward compatibility, but it was incomplete in the way it didn't introduce css class for the input cell. It is

[GitHub] struts pull request: WW-4634 Centre alignment doesn't seem to work...

2016-05-17 Thread lukaszlenart
Github user lukaszlenart commented on the pull request: https://github.com/apache/struts/pull/95#issuecomment-219829308 All the tags are implemented in FreeMarker, those archive templates are deprecated and should be thrown away. --- If your project is set up for it, you can reply

[GitHub] struts pull request: WW-4634 Centre alignment doesn't seem to work...

2016-05-17 Thread lukaszlenart
Github user lukaszlenart commented on the pull request: https://github.com/apache/struts/pull/95#issuecomment-219820654 So it's not a regression bug ;-) Yes, we do support Velocity. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub

[GitHub] struts pull request: WW-4634 Centre alignment doesn't seem to work...

2016-05-17 Thread victorsosa
Github user victorsosa commented on the pull request: https://github.com/apache/struts/pull/95#issuecomment-219820007 @lukaszlenart the parameter align was remove on this commit https://github.com/victorsosa/struts/commit/a0b34806b0f700a1fd240b698c4cec474711bbd0 --- If your

[GitHub] struts pull request: WW-4634 Centre alignment doesn't seem to work...

2016-05-17 Thread aleksandr-m
Github user aleksandr-m commented on the pull request: https://github.com/apache/struts/pull/95#issuecomment-219820070 Issue is about Velocity tags, not FreeMarker. Are we still supporting Velocity tags? --- If your project is set up for it, you can reply to this email and have your

[GitHub] struts pull request: WW-4634 Centre alignment doesn't seem to work...

2016-05-17 Thread lukaszlenart
Github user lukaszlenart commented on the pull request: https://github.com/apache/struts/pull/95#issuecomment-219817155 I'm just wondering what was the main reason, I have prepared a unit test and it didn't pass when using 2.3.29-SNAPSHOT which means 2.3.28.1 is broken the same way.

[GitHub] struts pull request: WW-4634 Centre alignment doesn't seem to work...

2016-05-17 Thread victorsosa
Github user victorsosa commented on the pull request: https://github.com/apache/struts/pull/95#issuecomment-219813310 CSS class to this table cell is what we have now and to have back compatibility as @lukaszlenart said. --- If your project is set up for it, you can reply to this

[GitHub] struts pull request: WW-4634 Centre alignment doesn't seem to work...

2016-05-17 Thread aleksandr-m
Github user aleksandr-m commented on the pull request: https://github.com/apache/struts/pull/95#issuecomment-219809968 Why not just add some CSS class to this table cell? Then end-users can style it as they want and align parameter will not be needed. --- If your project is set up

[GitHub] struts pull request: WW-4634 Centre alignment doesn't seem to work...

2016-05-17 Thread victorsosa
Github user victorsosa commented on the pull request: https://github.com/apache/struts/pull/95#issuecomment-219713305 Another thing I will change the name to just > align-center because this css class can be used in other places that are using the align attribute;

[GitHub] struts pull request: WW-4634 Centre alignment doesn't seem to work...

2016-05-17 Thread victorsosa
Github user victorsosa commented on a diff in the pull request: https://github.com/apache/struts/pull/95#discussion_r63517163 --- Diff: core/src/main/resources/template/xhtml/controlheader.ftl --- @@ -21,5 +21,8 @@ */ --> <#include

[GitHub] struts pull request: WW-4634 Centre alignment doesn't seem to work...

2016-05-17 Thread victorsosa
Github user victorsosa commented on the pull request: https://github.com/apache/struts/pull/95#issuecomment-219709110 Yes, it fix the main issue and keep the back compatibility. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as

[GitHub] struts pull request: WW-4634 Centre alignment doesn't seem to work...

2016-05-17 Thread lukaszlenart
Github user lukaszlenart commented on a diff in the pull request: https://github.com/apache/struts/pull/95#discussion_r63516015 --- Diff: core/src/main/resources/template/xhtml/controlheader.ftl --- @@ -21,5 +21,8 @@ */ --> <#include

[GitHub] struts pull request: WW-4634 Centre alignment doesn't seem to work...

2016-05-17 Thread victorsosa
Github user victorsosa commented on a diff in the pull request: https://github.com/apache/struts/pull/95#discussion_r63515782 --- Diff: core/src/main/resources/template/xhtml/controlheader.ftl --- @@ -21,5 +21,8 @@ */ --> <#include

[GitHub] struts pull request: WW-4634 Centre alignment doesn't seem to work...

2016-05-17 Thread lukaszlenart
Github user lukaszlenart commented on the pull request: https://github.com/apache/struts/pull/95#issuecomment-219706843 This doesn't fix the main issue, right? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your

[GitHub] struts pull request: WW-4634 Centre alignment doesn't seem to work...

2016-05-17 Thread lukaszlenart
Github user lukaszlenart commented on a diff in the pull request: https://github.com/apache/struts/pull/95#discussion_r63514815 --- Diff: core/src/main/resources/template/xhtml/controlheader.ftl --- @@ -21,5 +21,8 @@ */ --> <#include

[GitHub] struts pull request: WW-4634 Centre alignment doesn't seem to work...

2016-05-17 Thread victorsosa
GitHub user victorsosa opened a pull request: https://github.com/apache/struts/pull/95 WW-4634 Centre alignment doesn't seem to work in Velocity tags Fix for WW-4634 Centre alignment doesn't seem to work in Velocity tags You can merge this pull request into a Git