Skip to content

Allow format to return a DOM #106

Closed
lephyrius opened this Issue Apr 23, 2011 · 17 comments

2 participants

@lephyrius

Allow the format function to return a jQuery DOM instead of a string. So that it could easily be used with jQuery Template.

@fnagel
Owner
fnagel commented Apr 26, 2011

I'm not very happy with the current format functionality but its a less used feature -- I never used it so far. So, sadly, this is a low priority issue for me. If you like to help out I will be happy to implement your changes.

@lephyrius

Ok, so how do I commit a patch. I got one really nice solution to this.
This is the main reason I use this selectmenu so it is important to me.

@fnagel
Owner
fnagel commented May 23, 2011

Sorry about the long waiting time, but I had an surgery and was not able to work on the selectmenu widget the last few weeks.

There is a way of push / pull requests via GitHub. For this you need to have a repo and I need to pull the changes. Im not so familiar with this but I will give it a try if you like. Most time I patch my local file manually but even if i do so: GitHub has a quite nice "per-line" commenting system if you send me a pull request. This is great for discussing about your code!

But you could upload the changes as plain file and I will take a look at it if this is easier for you.

Would you please provide me some infos about jQury Template and where it is used so far. Is it a offical jQuery plugin?

@lephyrius

I explain it with some code and yes I use the official jQuery Template plugin:

$.template('selectitem', '<span class="ui-selectmenu-item-header">${name}</span><span class="ui-selectmenu-item-footer">${description}</span>' );

//...

  $('#cool_list').selectmenu({
            format: function (text) {
              var json_data = $.parseJSON(text);

              return $.tmpl('selectitem',json_data);
            }
          });

Here is a patch:

https://gist.github.com/988208

@fnagel
Owner
fnagel commented May 24, 2011

Nice! I will give it a try as soon as v1.1 is released. Tagged for 1.2 milestone.

@fnagel
Owner
fnagel commented Aug 1, 2011

retagged as I need to release a bugfix version soon.

@fnagel
Owner
fnagel commented Aug 2, 2011

Is it possible you attached the wrong gist? 988208 seems to be another issue. Just a nice cleanup in element creation. Thanks for that!

ps: try to press key "m" for markup explaination!

@fnagel
Owner
fnagel commented Aug 2, 2011

Ive read trough the manual of template and implemented your patch. Your posted snippet seems not to work as the json is invalid. Im not quite sure how to do this.

@lephyrius

I have tested this in the version on github.(current)
It works to return a DOM from the format function.

@fnagel
Owner
fnagel commented Aug 8, 2011

Mhh but var text within this context:

format: function (text) {
              var json_data = $.parseJSON(text);

is a HTML string which can not be transformed into json without regex parsing. Perhaps you forgot to mention some more changes within yout gist?
Perhaps you can post a fiddle with the needed changes?

@lephyrius

http://jsfiddle.net/ACRjA/1/

Here is a full jsFiddle for my usecase.

@fnagel
Owner
fnagel commented Aug 9, 2011

Ahh, the options contain a json string. Thats a nice solution when JS is enabled. What about the backfall?
Perhaps we should implement an option to pass the needed json as an option. What do you think?

@fnagel
Owner
fnagel commented Oct 10, 2011

Any feedback on this?

@fnagel
Owner
fnagel commented Nov 6, 2011

Any feedback on this?

@lephyrius

Thats a nice solution when JS is enabled. What about the backfall?

I have not thought of that. Maybe we could use HTML5 data-attributes like: 'data-json' .

Perhaps we should implement an option to pass the needed json as an option. What do you think?

That would be a nice idea. I cannot think of any other usecase for it.

@fnagel
Owner
fnagel commented Jan 16, 2012

Closed as no feedback was given. Please reopen if needed.

@fnagel fnagel closed this Jan 16, 2012
@fnagel
Owner
fnagel commented Aug 30, 2012

Please note: this issue seems to be solved within this pull request: #257

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.