WildcardSearch / Advanced-Sidebox

A plugin for MyBB forums that displays custom boxes on various forum pages.
GNU General Public License v3.0
20 stars 10 forks source link

AJAX doesn't consider expanders #138

Closed Destroy666x closed 10 years ago

Destroy666x commented 10 years ago

If an expander is collapsed, AJAX works and the box content is appearing regardless of closed status. Moreover, I think that collapse cookies for boxes should be checked before AJAX is used - I don't think that someone who has them collapsed needs the refresh. It can save some resources.

So it could work like that: cookie check - not collapsed, refresh cookie check - not collapsed, refresh user collapses box cookie check - collapsed, do nothing cookie check - collapsed, do nothing user opens box cookie check - not collapsed, refresh etc.

One more issue - boxes can't be expanded/collapsed after their AJAX had been ran.

WildcardSearch commented 10 years ago

Hmmm . . . good find. I'll get right on this. Thanks

WildcardSearch commented 10 years ago

One more issue - boxes can't be expanded/collapsed after their AJAX had been ran.

I'm not experiencing that. Which browser are you using?

Destroy666x commented 10 years ago

All browsers. To be more specific, it happens only if content was updated.

WildcardSearch commented 10 years ago

Something is odd about this. I cannot reproduce an error with this.

Let me finish the current update and try with that version if you have time.

Destroy666x commented 10 years ago

Ah, sorry, my bad, I forgot that I use smooth collapse jQuery script on that test forum. Every collapsible in MyBB works well with it except the case when AJAX is successful on boxes - weird. I disabled it temporarily and the bug disappeared. So it's not really ASB's issue. Still, out of curiosity (so I can spot the conflict) - does ASB change anything in expander's code during AJAX refresh at all?

But the 1st part of issue is still actual - also AJAX shouldn't be ran if the side arrows collapse one side, forgot to mention.

WildcardSearch commented 10 years ago

When the update happens, the side box's tbody gets replaced-- that is all.

And to the main issue here, I will have to check and see what the best way to handle that is. Glad there isn't a bug in the update JS :wink:

WildcardSearch commented 10 years ago

That should get it. Thanks for the report.

Good eye! :+1:

Destroy666x commented 10 years ago

It still seems to run AJAX, no matter if whole side or a single box is collapsed.

Destroy666x commented 10 years ago

Changed the container.style.display conditional to more accurate and reliable: if (this.container.offsetWidth <= 0 && this.container.offsetHeight <= 0) and it seems to work correctly in both cases now.

WildcardSearch commented 10 years ago

Okay I will make the change today.

Making some updates to the ACP side JS today and I'll get that in there as well.

WildcardSearch commented 10 years ago

Can we consider this done @Destroy666x ?

Destroy666x commented 10 years ago

I think so.

WildcardSearch commented 10 years ago

I just happened to think, maybe that should be || instead of &&?? Either way we don't want to waste execution.

Destroy666x commented 10 years ago

Not quite sure, I read somewhere that both need to be checked to be 100% certain that the element is hidden and so it works on all browsers. Some javascript libraries implement it like this, for example jQuery and its :hidden/:visible selectors: https://github.com/jquery/jquery/blob/master/src/css/hiddenVisibleSelectors.js

WildcardSearch commented 10 years ago

Okay great. That eases my mind. :+1: