Table, ValueChangeEvent & Labels

Hi !

I’m have a little problem - and as usual, I’m not sure if it is a bug, a normal behavior, or something I’m doing wrong :slight_smile:

I have a Table. In that Table, in order to control & have different CSS on fields, I use Labels instead of Strings, in some of my cells.

When the user click on a line, that row is selected only if he clicked on a cell that is not a Label.

While I understand why that happen (it is not the line that have been clicked, but the element inside, and it has priority on the event, which is fine if the said element is a Button, or a Select) it is, well, quite problematic in my case.

So ! Question :slight_smile:
Is there an other way to control the CSS appearance of some of my cells, other than using a Label ?
If not, how can I convince my Labels to transmit their click event to the Table ?

Thank you !

Quentin.

Hope this helps:
http://dev.itmill.com/ticket/870
(at least it works for row / items)

Sorry to dissapoint you, but that ticket/improvement was only done for the version 4 of IT Mill Toolkit, and is not currently supported in version 5 (I’m safely assuming you’re using v.5).

But to get back to your original problem, using Labels to add specific styles to the cell is IMO a good solution, but the fact that click events get captured in the Label is, again IMO, a bug (and I think it is because our Label implementation uses gwt HTML component which indeed captures all events).

If I were you I would create a ticket and see what happens :wink:

Hi!

I wont be adding a hack to pass clicks from ILabel to its parent. It is just not the way things have been designed. So I can tell you beforehand that I will most likely close the ticket as invalid.

Best option here is to reopen the ticket Jani pointed out and implement the feature for version 5 too. This is really needed. But to get this into release version will take some time.

Another option is to use “ClickableLabel” aka Button instead of Label and hook its click listener to do selection on server side. Styling button element is rather hard on IE6 (maybe on 7 too), so a custom client side component may be needed, but it should be easy to implement by extending GWT’s CustomButton component.

[quote=Matti Tahvonen]
I wont be adding a hack to pass clicks from ILabel to its parent. It is just not the way things have been designed.
[/quote]I’m not proposing a hack. Why not just implement the ILabel so it bubbles events like normal HTML elements do? Or does GWT do some really odd event handling automatically?

[quote=Matti Tahvonen]
So I can tell you beforehand that I will most likely close the ticket as invalid.
[/quote]I already knew this when proposing :slight_smile:

And please, people closing tickets: acknowledge the difference between an invalid ticket and a "wontfix" issue. In this case, the issue is valid, but the fix is not suitable.

It’s Table, not the label that prevents the selection; it does not react to clicks on cells that contain Components, because it can not know if that’s ok or not.
(Label actually passes the event trough just fine)

AFAIK we could actually implement this “the other way around”, so that table reacts to all clicks but the components are responsible for cancelling the event if needed. This would be a rather big change though, and would probably cause regressions…


How about this:

We could make Label a special case, since it usually does nothing when clicked (“usually”: we still need to cancel if the target is a link).
What do you guys think?

[quote=Marc Englund]
It’s Table, not the label that prevents the selection; it does not react to clicks on cells that contain Components, because it can not know if that’s ok or not.
(Label actually passes the event trough just fine)
[/quote]Doh, of course. Didn’t think this through properly.

But when would the solution you proposed be a problem (“the other way around”)? Shouldn’t all our current components cancel events when necessary? A label containing a link (A-element) is IMO a special case, which we shouldn’t handle in any way. The programmer can place needed event cancelling code to the element inline attributes (though ugly, should work), if needed.

I would very much like to avoid any major regressions, like most of us I think, so let’s just create a ticket from this and let it sink in for a while before we do anything.

And if the setItemStyleProperty-method is considered useful, reopen that ticket for v.5.

Marc, good idea. Label is already a special case in Table, so this might not even be too confusing…

While we’re on the subject: the original problem was that these is no convenient way to style individual cells - this is a feature that is requested from time to time (not very often, since you can work around it, but still) so perhaps we should just go ahead and add it.

We had a short discussion here at R&D, and one proposed solution would look something like this:

table.setCellStyleGenerator(new Table.CellStyleGenerator() {
   public String getStyle(Object itemId, Object propertyId) {
      if (itemId == SOME_ROW && propertyId == SOME_COLUMN) {
           // this is the cell I want to style
           return "aclassname";
      }
   }
});

This would be fairly straightforward to implement.

Thoughts?

//Marc

+1

(Should be put in 5.3.x)

That would be perfect for me :slight_smile:

Sorry for the time it took me to respond !

Do you have any idea on a ETA on that functionnality ?
I have several very simple tables, with only one fields that I have to style differently according to some objects attributes, and I have to be able to make selection in that tables…

Or maybe I can help implement that ?
As I’m a bit in a rush during my day work, I do not have a lot of time at the moment for that, but if you can point me to where the changes must be made, I can try (hey, I stille have some hours, during the night, from 4 to 6 am. where I do nothing but sleeping :))

Once more, thank you very much for your help & involvement :slight_smile:

Quentin.

We try to kill the rest of the bugs in 5.2, declare it stable and then start to work on new features that will be released in 5.3 series. So no ETA yet, sorry.

Sure! I would propose the following strategy:

  • Cut-n-paste Table.java to your project and package
  • Do not worry if the table implementation looks complex - you only have to modify paintContents (and it is fairly simple) :)
  • Only add one method “setCellStyleGenerator” and one inner interface “Table.CellStyleGenerator”
  • Try to keep all changes (including code formatting) minimal
  • Create a ticket about this enchantment
  • Attach diff between the original table and your implementation to the ticket
  • Use your.package.Table with 5.2.x in your application before 5.3.0 is released

We’ll try to include the patch to trunk asap - if it looks good :) This way your implementation will end up in 5.3.0.

Hmm, ok, I’ve tried looking into that.

Problem is, it seems that modifiing only Table.java is not sufficient.
The client side component IScrollTable, in the addCell method, doesn’t care about any “style” element in the UIDL, and just set " DOM.setElementProperty(container, “className”, CLASSNAME+ “-cell-content”); " …

Am I right, or looking in an entirely wrong direction ? ^^

I will try to modify IScrollTable too - but that is a totally different game, as it imply to change a default widgetset, and to rebuild, well, a lot of things :slight_smile:
Maybe, for my tests, using a new widget extending IScrollTable would be easier, I don’t know.

Quentin.

[quote]
Problem is, it seems that modifiing only Table.java is not sufficient.
The client side component IScrollTable, in the addCell method, doesn’t care about any “style” element in the UIDL, and just set " DOM.setElementProperty(container, “className”, CLASSNAME+ “-cell-content”); "
[/quote]You’re right on this one, unfortunately you have to modify the client-side implementation as well.[quote]
Maybe, for my tests, using a new widget extending IScrollTable would be easier, I don’t know.
[/quote]Hmm, I don’t really know about that. Extending the IScrollTable could result in some pretty sluggish performance and even strange behaviours, if things aren’t done in the right order. It’s a beast only Matti here has real knowledge of :slight_smile:

But, I mean not to discuorage you from trying, just saying that it might take a while to understand what needs to be done :slight_smile:

^^’

Well, ok, thank for your advice :slight_smile:
I’ll grab the full project & try to make my own build of the toolkit. I feel that won’t be so easy ^^

Quentin.

Great to hear I didn’t kill your spirit!

And remember, no issue is too small or trivial to ask about on this forum, we’re here to help!

\o/

It’s done & working :slight_smile:

Doing the actual modification to Table.java & IScrollTable.java was trivial - but managing to build the jar and use it correctly has been a bit more complicated ^^ Especially as I’m not a Eclipse user (Netbeans rules ! :p).

I will do the SVN patch either in the afternoon or on monday, and add it to Trac ASAP :slight_smile:

Quentin.

EDIT: Patch & Ticket done.
http://dev.itmill.com/ticket/1857
I hope I did everything right :slight_smile:

+1 for adding this to next version

Actually it is :) - see: http://dev.itmill.com/browser/trunk/build/readme.txt

Just reviewed the patch - looks great. The actual bug I found was missing requestRepaint() from the cell style generator setter.

Anyway - the feature is now integrated to trunk.

And in case you want to take a break from your actual project, feel free to submit more patches :wink: