Github user aleksandr-m commented on the pull request:
https://github.com/apache/struts/pull/95#issuecomment-221976394
Looks fine. Good job.
---
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 th
Github user victorsosa commented on the pull request:
https://github.com/apache/struts/pull/95#issuecomment-221804220
Yes, I am ok
---
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
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 well
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 f
Github user victorsosa closed the pull request at:
https://github.com/apache/struts/pull/96
---
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 and wishes so, or if the feature is e
Github user victorsosa commented on the pull request:
https://github.com/apache/struts/pull/96#issuecomment-221600945
PR remove, it wasn't requested
---
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
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 you
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 user lukaszlenart commented on the pull request:
https://github.com/apache/struts/pull/95#issuecomment-221556014
Tests are failing :(
```
Error Message
expected:<...bel: but
was:<...bel:
Stacktrace
junit.framework.ComparisonFailure:
expected:<...be
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 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 w
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 attrib
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 sol
Github user victorsosa commented on the pull request:
https://github.com/apache/struts/pull/96#issuecomment-221015218
OK
---
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 a
Github user lukaszlenart commented on the pull request:
https://github.com/apache/struts/pull/96#issuecomment-220948751
That's why people should migrate to 2.5 ;-) I think we can ask reported if
he wants the same in 2.3
---
If your project is set up for it, you can reply to this emai
Github user victorsosa commented on the pull request:
https://github.com/apache/struts/pull/96#issuecomment-220947364
But it is using the parameter align instead of a css class. the align
parameter is deprecated in html 5.
---
If your project is set up for it, you can reply to this e
Github user lukaszlenart commented on the pull request:
https://github.com/apache/struts/pull/96#issuecomment-220900444
As far I understand the issue was in 2.5 only, 2.3 works as expected.
---
If your project is set up for it, you can reply to this email and have your
reply appear on
GitHub user victorsosa opened a pull request:
https://github.com/apache/struts/pull/96
WW-4634
Cherry-pick PR for support-2-3
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/victorsosa/struts support-2-3
Alternatively you can rev
Github user victorsosa commented on the pull request:
https://github.com/apache/struts/pull/95#issuecomment-220818804
@lukaszlenart ok
---
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 fea
Github user lukaszlenart commented on the pull request:
https://github.com/apache/struts/pull/95#issuecomment-220817379
@victorsosa this commit a0b34806b0f700a1fd240b698c4cec474711bbd0 dropped
`align` attribute for buttons, it shouldn't affect `textfield` tag. Anyway, I
think we shoul
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 user victorsosa commented on the pull request:
https://github.com/apache/struts/pull/95#issuecomment-220740769
It will break back compatibility as @lukaszlenart said.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well
Github user aleksandr-m commented on the pull request:
https://github.com/apache/struts/pull/95#issuecomment-220522532
@victorsosa I mean something like `` in
controlheader.ftl and `.tdInput {text-align:left;}` in styles.css. <-- This is
for 2.5.
And if alignment doesn't work
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 to
Github user aleksandr-m commented on the pull request:
https://github.com/apache/struts/pull/95#issuecomment-219829978
@lukaszlenart Ahh ok. Thanks for clarifying that.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If y
Github user victorsosa commented on the pull request:
https://github.com/apache/struts/pull/95#issuecomment-219826548
the css class for that cell is in the controlheader.ftl already.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub
Github user victorsosa commented on the pull request:
https://github.com/apache/struts/pull/95#issuecomment-219826190
OH ok, got you
https://github.com/apache/struts/tree/master/core/src/main/resources/template/archive
I miss that.
---
If your project is set up for it, yo
Github user aleksandr-m commented on the pull request:
https://github.com/apache/struts/pull/95#issuecomment-219824169
Yep, the aforementioned commit misses css class for that cell in the
`controlheader.ftl`. It should be the same way as for the buttons. :)
---
If your project is set
Github user aleksandr-m commented on the pull request:
https://github.com/apache/struts/pull/95#issuecomment-219823160
How the Velocity templating is working? The only Velocity templates I see
are in the
[archive](https://github.com/apache/struts/tree/master/core/src/main/resources/te
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 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 pro
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 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 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 em
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 fo
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; wh
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
"/${parameters.templateDir}/${para
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 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
"/${parameters.templateDir}/${pa
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
"/${parameters.templateDir}/${para
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 proje
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
"/${parameters.templateDir}/${pa
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 repos
43 matches
Mail list logo
