Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Corrected example JSON for grouped results #53

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lesiki
Copy link

@lesiki lesiki commented Jan 30, 2013

The js code example for grouped results expects groups to have a 'name', not a 'text', as provided in the JSON example. I slipped up on this as I built my server-side code to feed JSON matching the example.

@michaelperrin
Copy link

I tested it again and the text key seems to be the right one.

Here is a (silly) code example which should prove it:

$("#example-input").ajaxChosen({
    type: 'GET',
    url: '/ajax-chosen/grouped.php',
    dataType: 'json'
}, function (data) {
    var results = [{
        group: true,
        text: "Europe",
        items: [
            { "value": "10", "text": "Stockholm" },
            { "value": "23", "text": "London" }
        ]
    },
    {
        group: true,
        text: "Asia",
        items: [
            { "value": "36", "text": "Beijing" },
            { "value": "20", "text": "Tokyo" }
        ]
    }];

    return results;
});

This piece of code works fine with the "text" entries.

But that's right that when I made a PR concerning item orders and had to work a bit with the Ajax Chosen code and updated the documentation, I found that "text" is not the best JSON key name, "name" could be a better choice.

@lesiki
Copy link
Author

lesiki commented Jan 31, 2013

Sorry, may not have been clear enough in my description. I agree entirely that each group in 'results' should have a 'text' key, but the sample javascript that populates 'results' expects the server to return a 'name' key. The JS in question is on line 101:

text: val.name, // label for the group

This means that when I designed my server-side code to return JSON that already matches the format expected in 'results', the sample Javascript code would failed because it expected a 'name' key instead of 'text' key. I copied this sample Javascript as is, and therefore had to change my server-side code to return 'name' instead of 'text'.

Maybe a more appropriate correction is to correct the sample JS to the following?

text: val.text, // label for the group

@michaelperrin
Copy link

Sorry that I didn't get it right. You're right that the example may be a bit confusing about the server response.
I checked and realized that the part of the documentation about the server side response is not mine, so it will be up to the author to decide. Maybe you can change your PR to reflect the change you mention in your last comment?

@lesiki
Copy link
Author

lesiki commented Jan 31, 2013

Thanks, made the change as suggested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants