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

Add item data to awesomplete-select event #16835

Conversation

vlazar
Copy link
Collaborator

@vlazar vlazar commented Jan 30, 2016

I made this PR as minimal as possible just to add new property to awesomplete-select event.

However I think if this one is merged, later we should expose this._suggestions as public this.suggestions property. It's a list of current matched items data. With just this 1 additional property we can do the following in instance methods:

// get current selected item data
this.suggestions[this.index];
// the same way as we can do now to get current selected item DOM element
this.ul.children[this.index];

Of course we can go further and add public instance properties to match suggested event properties in your comment #16821 (comment)
This will simplify code and be more convenient public API.

// same as above
this.data;
this.element;

// or since we already have this.selected:
this.selectedData;
this.selectedElement;

@vlazar
Copy link
Collaborator Author

vlazar commented Feb 1, 2016

@LeaVerou I believe now I have a very good implementation for data support in different formats (key/value objects, arrays, etc.). But I need this.data or this.suggestions or at least this._suggestions from here to move further with small simple PRs.

@TxHawks
Copy link
Contributor

TxHawks commented Feb 1, 2016

O'm also waiting for this to update #16795 accordingly

@vlazar vlazar closed this Feb 1, 2016
@vlazar vlazar force-pushed the feature/add-item-data-to-awesomplete-select-event branch from da7573c to dd4db90 Compare February 1, 2016 09:43
@vlazar vlazar reopened this Feb 1, 2016
@vlazar vlazar closed this Feb 1, 2016
@vlazar vlazar force-pushed the feature/add-item-data-to-awesomplete-select-event branch from 48ca757 to dd4db90 Compare February 1, 2016 09:49
@vlazar vlazar reopened this Feb 1, 2016
@vlazar
Copy link
Collaborator Author

vlazar commented Feb 1, 2016

@TxHawks Please don't update your #16795 yet when (and if) this PR is merged. The next PR I have in mind would make the whole key/value story much simpler.

If all goes as I imaging it, we can split your PR into 2:

The one that adds selected element property to awesomplete-select event. It looks like Lea is in favor of element as property name for that one. I guess we should skip awesomplete-selectcomplete event for now to finalize all property names first.

The second one with example of how to work with array of objects. This one can be totally unrelated to added event properties if my second PR will be accepted. Or maybe it can show the usage of event properties in addition to just using an array of objects.

Smaller PRs have better chance to be merged soon and this will cleanup the commit history of #16795 since it looks like the final implementation will look very different from what it was in the beginning.

@TxHawks
Copy link
Contributor

TxHawks commented Feb 1, 2016

@vlazar 👍

@LeaVerou
Copy link
Owner

LeaVerou commented Feb 5, 2016

Ok with merging once the helper function is added on $ (and renamed to siblingIndex).

vlazar added a commit that referenced this pull request Feb 5, 2016
…plete-select-event

Add item data to awesomplete-select event
@vlazar vlazar merged commit 332ca53 into LeaVerou:gh-pages Feb 5, 2016
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.

3 participants