Skip to content

Add item data to awesomplete-select event - #16835

Merged
vlazar merged 2 commits into
nudeui:gh-pagesfrom
vlazar:feature/add-item-data-to-awesomplete-select-event
Feb 5, 2016
Merged

Add item data to awesomplete-select event#16835
vlazar merged 2 commits into
nudeui:gh-pagesfrom
vlazar:feature/add-item-data-to-awesomplete-select-event

Conversation

@vlazar

@vlazar vlazar commented Jan 30, 2016

Copy link
Copy Markdown
Collaborator

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

vlazar commented Feb 1, 2016

Copy link
Copy Markdown
Collaborator Author

@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

TxHawks commented Feb 1, 2016

Copy link
Copy Markdown
Contributor

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

vlazar commented Feb 1, 2016

Copy link
Copy Markdown
Collaborator Author

@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

TxHawks commented Feb 1, 2016

Copy link
Copy Markdown
Contributor

@vlazar 👍

@LeaVerou

LeaVerou commented Feb 5, 2016

Copy link
Copy Markdown
Collaborator

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 nudeui: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