container function to generate container element #17143
Conversation
| className: "awesomplete", | ||
| around: input | ||
| }); | ||
| this.container = this.container(input); |
There was a problem hiding this comment.
Do note that this means the this.container() function is overwritten and not accessible anymore. Are we sure we don't need to access it again?
There was a problem hiding this comment.
If I am reading the original code correctly, the container element is never re-created during the object life cycle. That's why I've let the function to be overwritten. I can change the field used for the container function - do you have a preference for the field name?
Thank you for looking at this PR.
There was a problem hiding this comment.
Nah, I think container is a good name. Perhaps we can store the constructor options somewhere? Like this.options? Then we still have access to the function, so you can compare it to the default function in the check in destroy() instead of the less foolproof way you're checking now.
There was a problem hiding this comment.
Done. Added this.options to hold the original options passed to Awesomplete constructor. Checking this.options.container in destroy() and cleaning up the container element only when this.options.container is not set i.e. when container is originally created by Awesomplete.
awesomplete.js
Outdated
| @@ -203,11 +201,13 @@ _.prototype = { | |||
| $.unbind(this.input, this._events.input); | |||
| $.unbind(this.input.form, this._events.form); | |||
|
|
|||
| //move the input out of the awesomplete container and remove the container and its children | |||
| var parentNode = this.container.parentNode; | |||
| if (this.input.parentNode === this.container) { | |||
There was a problem hiding this comment.
What's the reasoning behind this if?
There was a problem hiding this comment.
The container element should only be destroyed by the Awesomplete if it was created by the Awesomplete - the condition of the if is true when the container element was created by the default method of creating it.
There was a problem hiding this comment.
But see my other comment about improving the check.
There was a problem hiding this comment.
Basically, what do we want if the container is custom, but also a parent of the input? Do we want to destroy it or not? That affects what we check for here.
addresses a use case where container element cannot reuse input element parent because input parent clips container content also solves LeaVerou#17086 & LeaVerou#16718
melitele
force-pushed
the
container-config
branch
from
4b6e170 to
bf33271
Compare
|
Nice, merged! Thank you so much! |
|
Thank you for taking my PR. And thank you for making and sharing this component. |
I've added a container function to options that will allow client to provide a different container element. The default version of container function creates container element as before. I believe this is in line with the item function that customizes creating item elements.
This addresses my use case where container element cannot reuse the input element parent
because the input parent clips container's content. It also addresses a case when input element has to be next to it's original siblings.
It also solves #17086 & #16718
Thank you for consideration.