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

"recursive" append & setAjaxLoadedContent #1

Open
speto opened this issue Oct 18, 2016 · 0 comments · May be fixed by #2
Open

"recursive" append & setAjaxLoadedContent #1

speto opened this issue Oct 18, 2016 · 0 comments · May be fixed by #2

Comments

@speto
Copy link

speto commented Oct 18, 2016

Hello,
there seems to be a problem with your last commit and I'm not sure about your code intentions on lines 90-95

Let's consider example below:

<div id="elements">
    <item />
    <item />
    <item />
</div>

After loading more content on line 88 you find and extract another <div id="elements"> from response and put it at the end so finally it looks like:

<div id="elements">
    <item />
    <item />
    <item />
    <div id="elements">
        <item />
        <item />
    </div>
</div>

It will be probably better to append not whole container, but only child elements on line 95:

$(options.elementsSelector).append(elements.html());

Am I missing something? If not, just let me now, I can send you pull request. And If it's possible please provide more detailed example of knp.setAjaxLoadedContent(html); use.

I can understand setAjaxLoadedContent -> update -> getNextPage, but update seems to be uncomplete and condition:

//there is #elements_selector div in the response, which means
//setAjaxLoadedContent is used

in ajax callback on lines 90-95 doesn't make sens to me, because whole clickListener is bind only to click event on loadMoreButton so it can't be reached after calling of setAjaxLoadedContent.
If you can, just let me know what did you try to achieve and maybe I would help with it.

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 a pull request may close this issue.

1 participant