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 #654 (closed enhancement: fixed)

Opened 11 years ago

Last modified 10 years ago

[PATCH] Add hidden field to autocomplete

Reported by: xentac Owned by: anonymous
Priority: normal Milestone: 0.9a2
Component: TG Widgets Version:
Severity: normal Keywords:
Cc:

Description

We have found the need to add ids to the autocomplete widget. That way we know which database row the user selected, instead of trying to match a string.

Sadly, this breaks backwards compatibility when you first set up the form (the value dictionary has to change) and when you get data back from the form, other than that, the JSON search method can stay the same. All this patch does is add a second hidden field and change the name of the text field.

We ended up converting the AutoCompleteField into a CompoundWidget? to be able to do this properly.

Attachments

hidden_field.patch Download (6.2 KB) - added by xentac 11 years ago.
New patch for r913
autocomplete.patch Download (3.2 KB) - added by michele 11 years ago.
patch
autocomplete-take2.patch Download (3.3 KB) - added by michele 11 years ago.
again
autocomplete-take3.patch Download (3.3 KB) - added by michele 11 years ago.
Should I play golf instead?

Change History

comment:1 Changed 11 years ago by godoy

And what about people that are not using it to any FK or something like that? For example, I use it to make it easy for people to enter similar text data. I have UnicodeCols? in my model and there are some of them that are suitable for AutoComplete?. So, to help users entering data and help standardizing things I use AutoCompleteWidget?. These columns aren't suitable for another table... IDs would make no sense in this case.

What would probably solve the case, then, is another widget instead of changing the existing one.

comment:2 Changed 11 years ago by xentac

The part that I didn't explain very well is that the text is still accessible the same way as normal. It still accepts JSON data as a list of strings.

But, in the cases where it does make sense to add extra data, you can also get access to the extra data through a hidden field. It's entirely optional.

If you want to use it the same as you used it before, that's still possible. It's just that the field name has changed.

comment:3 Changed 11 years ago by michele

Mmm, there is some work going on (from me and Alberto) that will really reduce the boilerplate you had to write to make this work. :-(

For example the FieldSet? widget now works with just the template and it doesn't need an update_data method (yahi!), I hope we can commit it soon so you (or we) can adapt your patch.

comment:4 Changed 11 years ago by roger.demetrescu

Hi Michelle

What kind of changes are you doing ? Does it affect all the widgets or only the FieldSet? widget ?

The reason I'm doing this question is because I'm currently looking at all the available widgets as reference to build the Tabber widget.

Thanks ! :)

comment:5 Changed 11 years ago by michele

Hi Roger,

basically it affects only CompoundWidgets and the nested managing things (by removing it from display ;-)), I'm probably going to commit it really soon now since I've ported it to work with svn and in this way Alberto can add it's changes for #613 later on.

PS Michelle -> Michele I'm a "he" ;-)

comment:6 Changed 11 years ago by roger.demetrescu

Michelle,

No doubt you are a "he"... :)

Well, the Tabber and Tab widgets derive from CompoundWidget?... so I'll wait for you commmit before releasing a final version.

By the way, what is the really advantage of using the ContainerWidget? as a superclass of a widget ? As far as I've seen, there's no a single widget today that derives from this class... Do you a use case ?

comment:7 Changed 11 years ago by michele

Hi roger,

No, ContainerWidget? is not used by any one ATM, I will probably remove it, anyway my initial idea was to provide a way to pack some generic (not form fields) widget into only one and pass it to the template.

ciao ciao

PS For the Michele vs Michelle thing take a look  here :D

comment:8 Changed 11 years ago by roger.demetrescu

Hahaha... I understand Jorge Godoy.. in the beginning I thought you were a women. :)

Well, once we are having this conversation, let me ask you another question about widgets. If wou want to go to ML that's fine to me...

Id like to have something like 'fields' from Form's class, but with the possibility of mixing Widgets and strings, integers, etc. The logic would be something like:

  if thething is Widget:
      render the widget
  else:
      render the str(thething)

Is there an easy way of doing that ? Is it a wrong idea ? My first thought was allowing the Tab class to do things like:

  t = Tabber(tabs = [
                    Tab(title="hello", content="just a string"),
                    Tab(title="world", content=TextField("name"))
                    ], 
             defaulttab = "world")

Thanks again for been patient... :)

[]s Roger

comment:9 Changed 11 years ago by michele

Hi Roger,

[Yep, all comes from Jorge misunderstanding... :D]

I need to go offline now so I haven't put much thoughts on this, what you are trying to do can make sense (I think) but I'm not well tuned ATM :D I need to think a bit more about it.

Anyway I would guess you can just get away by assuming that in the content you're always receiving a widget, at the constructor in you're receiving a basestring you can wrap it inside a widget (for example DumpString?, just an example) and in the template this let's you always call display on content.

That's just an idea...

ciao ciao

(we are a bit off topic, if you have other questions ping me with an email, you can get my address using the top poster info on the group)

comment:10 Changed 11 years ago by michele

My changes are now in, take a look at r913.

Changed 11 years ago by xentac

New patch for r913

comment:11 Changed 11 years ago by roger.demetrescu

Hi xentac,

Your patch is using "@update_path" which is not compatible with Python 2.3. (you can see that the same thing happened at #650)

Cheers Roger

comment:12 Changed 11 years ago by michele

That's a problem with my code I forgot to remove @update_path and instead use the 2.3 notation.

I will take a look at xentac patch and commit it along with fixes for base.py and forms.py.

Thanks Roger and Jason.

comment:13 Changed 11 years ago by michele

Roger, I think you were refering to me, I can't see any use of update_path in xentac's patch that I'm going to commit.

comment:14 Changed 11 years ago by roger.demetrescu

Michelle, you're right.

I saw xentac's comment "New patch for r913" and I just clicked in the link. I realize now that I din't see xentac's patch, but your changeset instead.

Sorry xentac !! :(

comment:15 Changed 11 years ago by michele

  • Status changed from new to closed
  • Resolution set to fixed
  • Milestone set to 0.9a2

Committed in r918.

Thanks Jason!

Changed 11 years ago by michele

patch

Changed 11 years ago by michele

again

Changed 11 years ago by michele

Should I play golf instead?

comment:16 Changed 11 years ago by godoy

Take3 worked and I was able to fill information for the text part independently, so it got my CSS and attrs applied correctly.

Thanks.

comment:17 Changed 11 years ago by godoy

By the way, this should be applied soon... Should I reopen the ticket so that we don't forget it?

comment:18 Changed 11 years ago by michele

Nope, I'm going to commit it in one minute!

Thanks Jorge, glad it works. :-)

comment:19 Changed 10 years ago by tonis

That's a really nice addition to the autocomplete widget.

Note: See TracTickets for help on using tickets.