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

fix destroy method data-anchor #2790

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from

Conversation

qaz3954179
Copy link

Remove jquery.fn.data's cache for anchor when destroy.

@alvarotrigo
Copy link
Owner

Thanks for it!
Could you please explain the problem it fixes? And where in fullPage.js is used the data method to set the anchor value?

@qaz3954179
Copy link
Author

qaz3954179 commented Jul 10, 2017

In the same page, no reload.
Firstly, I invoke it, eg: "$.fullpage({anchor:[anchor_1, anchor_2]})"
Secondly, to destroy it, and invoke it again, with different anchor。 eg: $.fullpage({anchor: [anchor_sec_1, anchor_sec_2]})
when i scroll to the top, the window.location.hash's value is the first one. eg: url/#anchor_1.

@alvarotrigo
Copy link
Owner

Right I see now! Thanks for the explanation!
Although you won't need to use removeData. Removing the attribute should do it. removeAttr.A

And it should only do it when the anchors are defined in the fullpage.js initialization object, not when they are defined in the DOM with the data-anchor attribute. So we'll need a condition there or an extra flag to figure out if it was defined in one or another way.

@qaz3954179
Copy link
Author

Ah, I'm not sure, in that case, when they are not defined.
But, we can't use the removeAttr , we should to clean the cache with removeData, not remove the attribute.
Actually, the Dom' attribute is the new value, but, the $.fn.data method don't get it.

@alvarotrigo
Copy link
Owner

But, we can't use the removeAttr, we should to clean the cache with removeData, not remove the attribute.

Why.
The attribute should be removed if it was added by fullPage.js.
And it shouldn't when it wasn't. (defined by the user in the DOM)

@qaz3954179
Copy link
Author

oh, you are right.
I think, this is another question.
The solution to that question is like what you said.

need to use removeData
So we'll need a condition there or an extra flag to figure out if it was defined in one or another way.
i had re-push it.

@alvarotrigo
Copy link
Owner

You defined the flag to true by default. It shouldn't be like that.
You should probably look at it here in this line.

Let me know if you still want to commit the pull request to devor not.

Then add it in the previous loop:

container.find('[data-anchor]').each(function () {          
     $(this).removeData('anchor');
     if( anchorsDefinedByUser){
           //do whatever
     }
});

@alvarotrigo alvarotrigo changed the title fix destroy method fix destroy method data-anchor Oct 6, 2017
fix destory method data-anchor
@qaz3954179 qaz3954179 changed the base branch from master to dev October 18, 2017 09:08
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