Warning: Can't synchronize with repository "(default)" (Unsupported version control system "svn": No module named svn). Look in the Trac log for more information.

Ticket #902 (closed defect: wontfix)

Opened 13 years ago

Last modified 12 years ago

[PATCH] All input widgets should have a "field_id" param to allow overriding the DOM element's id

Reported by: alberto Owned by: alberto
Priority: normal Milestone: 1.0b1
Component: TG Widgets Version: 0.9a5
Severity: major Keywords:
Cc: michele

Description

This param should override the autogenerated id for the field. Without this functionallity it's very hard to refer to the input fields in JS code or store any kind of info in them.

The use case I'm trying to implement is to store a record's id in the id attribute like phone-6 which corresponds to the phone field for the record with id 6. I need this for the JS code to parse and build the URL where the value should be submitted.

I'm attaching a patch for review and inspiration of a better solution. ;)

P.S: Maybe this should apply to all widgets

Attachments

widget_field_id.patch Download (1023 bytes) - added by alberto 13 years ago.
widget_field_id.2.patch Download (2.6 KB) - added by alberto 13 years ago.
widget_field_id.3.patch Download (13.7 KB) - added by alberto 13 years ago.
How about this?

Change History

Changed 13 years ago by alberto

comment:1 Changed 13 years ago by anonymous

  • Cc michele added

comment:2 Changed 13 years ago by godoy

Even though I like this idea, I'd like to remember Roger's recent cleanup on widgets and the fact that to be *valid* (X)HTML we have to have id == name, otherwise we'd get warnings from validators.

I believe that id == name, so no new attribute is needed and we're generating valid (X)HTML output. I would love if all output from TG is valid because I'll know that any problems on my page can be fixed by me and are not my framework's fault.

Since we're tying id at instantiation time -- as we do with name -- I see no advantage to allowing it to be different because we'll already have duplicate names on the page if we have to ".display()" calls. As for accessing each individual field on a multi-form page, we can always qualify our forms (that should also have the same id as the name...).

So, I'm -1 for all this but +1 on fixing the generated id to be equal to the name of the widget.

comment:3 Changed 13 years ago by michele

@Alberto:

I reverted such a change 1 month ago I believe, the reason is that field_id can't change at display time since a Form widget needs to access it as an attribute of the field to make the label point to it, making it variable will introduce a discrepancy between the displayed id and the id attribute making a label useless. :-(

What about keeping the id as the value of an hidden field that you parse? can this work?

@Jorge:

I don't remember this name==id thing... and I'm not keen on changing this since it doesn't really make much sense IMHO. :D

comment:4 Changed 13 years ago by roger.demetrescu

@Michelle

I think Godoy is talking about this  thread (do a search for "The difference between NAME and ID only matters when"

@Alberto

I am not the expert here... but please be sure that this change won't make all widgets start generating duplicate ID's... This would be really bad (see  http://www.w3.org/TR/html4/struct/global.html#adef-id) :)

comment:5 Changed 13 years ago by roger.demetrescu

@Michele

I meant "@Michele", not "@Michelle"... :)

God !! When will I learn ?

comment:6 Changed 13 years ago by alberto

Thanks for the feedback :) !

Ok, how about this:

  • Widget accepts a new optional parameter at the *contructor*, called id. It's not a param as we know it which means it cannot be overriden at display. This should settle the discrepancies Michele was talking about.
  • If passsed, this id overrides the id generated by FormField._get_field_id. If not passed, everything works as it's been working 'til now. Backwards compatible! ;)

This would make possible to have duplicated ids if the programmer wants to shot himself on the foot ;)

I think this change should go to Widget because the id is very commonly used in JS code to refer to elements, even if they're not InputWidgets?, for example, let's say your have separate widgets in a page, being able to force/override an id makes it easy to connect a signal handler (in Mochi's terms) to one of them so other widgets can force it to update when they asyncronously change state in the app.

Apart from storing info in the id... which I'm using for an ultra-cool widget Oprius Foundations has inspired me ;)

Alberto

Changed 13 years ago by alberto

comment:7 Changed 13 years ago by michele

Quick since I'm going offline... :-(

The id is bound to the name of the widget so you can already override it in an easy way and in *one* way, the name is what makes the distinction between two widgets so I can't see why we need two different way (name and id) to distinguish them when by using different names you get different ids that you're sure will not cause any name collision.

Also, I don't think we should encourage developers to shoot on their foots with id collisions, if you have and id collision you can't distinguish widgets so the whole thing is useless.

Finally, adding field_id to the Widget base class makes no sense since a Widget base class should not make such an assumption as field_id (a field is a form related concept), a better name in this case will be "widget_id" or simply "id" (if possible (name collision with python? I haven't tested it) id makes even more sense for a field instead of field_id).

But in the end why we shouldn't use the name as the widget id? apart from form related widget the name HTML attribute has no use so you should probably use it as the id, this means you already have the feature you want without any modification.

class MyWidget(Widget):
   template = """<div id="$name">...</div>"""

Anyway if you want such a thing please consider using "id" or "widget_id" but not "field_id", in this case using "id" everywhere and deprecating "field_id" will be the best thing to do IMHO. ;)

comment:8 Changed 13 years ago by alberto

Anyway if you want such a thing please consider using "id" or "widget_id" but not "field_id", in this case using "id" everywhere and deprecating "field_id" will be the best thing to do IMHO. ;)

Forgot to mention that... Surely field_id looks awkward in a non-form Widget... I like id better, as it should set the id attr of the widget's root node.

Regarding the name/id issue, the problem is that I need to *force* an id on a widget, cannot let the form autogenerate it because I store the row's id on every field for that row for the JS code to parse.

So, maybe a crack-full idea, but here it goes:

  • "name" moves form Widget to FormField?, as only FormFields? (<input> or similar tags) have a "name" attribute associated to it's DOM elemement (right?).
  • Widget gets a new "id" parameter to set the id of the widget's root DOM element. This makes sense to me because any DOM element can have and "id"...
  • The "id" argument to the constructor is optional, if None is passed, an automatic id will be generated (like it's currently done)

Opionions?

Be sending a patch soon with this proposed changes...

comment:9 Changed 13 years ago by michele

Now I'm really going offline... :D

Anyway name is here to stay, you can't remove it since we need it to route parameters using value_for and params_for, also WidgetsList? wants to set a name.

Changed 13 years ago by alberto

How about this?

comment:10 Changed 13 years ago by alberto

How about this last one?

field_id -> id name remains the same. Behaviour as usual except if you pass an explicit 'id' at widget's construction time.

comment:11 Changed 13 years ago by alberto

Well, I've stumbled upon a wall I cannot cross unless:

a) Id can be overriden at display b) I instantiate new widgets on every request.

What I'm trying to accomplish is a cross between a fastdatagrid and a form. Let me explain: A form is defined with fields A, B and C. The form receives a SelectResults? as value and renders each record as a row whith columns A, B, C. These <td>s can be edited by clicking on them, which causes the <span> to become an input widget (as defined by the form fields) which gets submitted asyncronously when it loses focus.

As you can see, every input widget is rendered N times (where N is the number of rows) causing N id collisions :( One soulution, as I've mentioned, would be to override the id at display, another would be to generate the N*M widgets at form's display time (cloning them from the Form)... yuck! :(

Maybe I'm doing something crazy... am I?

If not, any idea how could I implement this without patching widgets? I guess that the only thing it'll be needed is to make 'id' overridable at *display* time. Is there any way to get around the label/id limitatation?

I'm beginning to think we went too far with supporting the nested widgets scenario (probably, err, most probably my fault) and this is becoming an obstacle in designing other types of widgets :(

Alberto

comment:12 Changed 13 years ago by godoy

@Michele

We can't "predict" the mangled output of "name" as a fixed ID for every case. If I use a widget inside a form, it will have one ID, if I use it by itself, it will have another, it I use it inside a fieldset inside a form, it will have yet another ID.

You have three cases depending on where you ".display()"/".render()" your widget and you can't use the same $('field_id') to access all of them.

Besides that, one has to render the page -- if it is too complex to use the shell... -- to find out what the ID will be.

These reasons make it very interesting to be able to specify an "id" attribute.

I'm also from the already stated opinion that we aren't babysitters and if the guy / girl wants to shoot him/herlself in the foot, let it be. Not repeating ids in an HTML page is something that is inherent to how HTML works, not how TG works...

comment:13 Changed 13 years ago by godoy

@Alberto

Repeating widgets doesn't work for you? Can't you write a custom template that receives the number of iterations and generates a "iteration:id" ID?

/me having his revenge...

I've already fought that fight and ended up with JavaScript? generating my table... But there were no repeating widgets by that time and I haven't looked into them yet.

comment:14 Changed 13 years ago by alberto

@godoy:

Yep, seems that I can (get away with RepeatingWidget?)! :) Thanks! (Be releasing this widget soon as a sign of gratitude ;)

IIRC, you're trouble with something similar (your revenge) was with the name attribute, which is parsed by TG to create the nested structure of input params so it cannot be overriden safely. The id is a different beast here ;) (read: unadmitted

However, I still think that the possibility of overriding 'id', at least at construction time, is a good idea... Even though I don't really need it for this now.

comment:15 Changed 13 years ago by alberto

  • Owner changed from anonymous to alberto

comment:16 Changed 13 years ago by kevin

  • Status changed from new to closed
  • Resolution set to wontfix

I'd like to hold off on doing this until a new, current use case shows up. Better to not add more switches and knobs if we can avoid it.

Note: See TracTickets for help on using tickets.