filamentgroup / Overthrow

A tiny, no-frills, framework-independent, targeted overflow: auto polyfill for use in responsive design.
MIT License
906 stars 94 forks source link

Sidescroller nav auto hiding not always working #51

Closed matthewbakerturner closed 11 years ago

matthewbakerturner commented 11 years ago

When the sidescroller UL container is large enough to hold all the items, the right arrow still appears

johnbender commented 11 years ago

@matthewbakerturner

Thanks for posting this issue!

I'm a bit confused what you mean when you say:

the sidescroller UL container is large enough to hold all the items

Are you seeing this in the demo here? What browser did you view it from?

Keep in mind that in the demos the arrow is always going to be visible the extension simply applies a disabled class which you can use to hide, dim, ignore clicks, etc.

matthewbakerturner commented 11 years ago

Sorry for not being clear, the Fridays have set in. :)

Basically, the condition is if no scrolling is needed, both nav arrows should have the disabled class. So in the case of the demo, if there are four slides or less, the right arrow should have the disabled class.

Does this make more sense?

Thanks Matt

johnbender commented 11 years ago

Matt,

Perfect, I see precisely what you mean in the fixed-width demo. I'll get right on it!

johnbender commented 11 years ago

Matt,

I've pushed a fix to the fixed-width branch that should resolve the issue for you. Give it a try and let me know!

matthewbakerturner commented 11 years ago

Sorry for the delay in reviewing.

The arrows hide now if all the slides fit but I noticed something new. If the arrows are hidden on load and then the browser is resized so some of the tiles are hidden, the right arrow does not reappear. The left arrow works as it should, automatically hiding and reappearing correctly. Hopefully this makes sense.

johnbender commented 11 years ago

Matt,

Yep it makes perfect sense. I'll take a look!

johnbender commented 11 years ago

Matt,

The navigation enable/disable now respects resize. Can you take a look?

matthewbakerturner commented 11 years ago

The resize looks good, found one other minor thing. When the carousel loads that needs scrolling, the right arrow loads as disabled. Resizing the carousel will re-enabled it.

Thanks Matt

johnbender commented 11 years ago

Matt,

Sorry for the response cycle here.

http://filamentgroup.github.io/Overthrow/examples/sidescroller/disabled-nav.html

That examples shows a sidescroller that needs scrolling. And the same for the fixed-width:

http://fixed-width.origin.overthrow-cnn.fgtest.com/examples/sidescroller/fixed-width.html

So I'll need some more detail on how to reproduce this. What browser are you using?

matthewbakerturner commented 11 years ago

I'm in Chrome. I'll get you some additional details tomorrow. The issue may very well be how I'm creating the carousel.

Thanks Matt

matthewbakerturner commented 11 years ago

Here's the HTML for the carousel on initial load. The width of the <ul> is calculated in javascript. Each <li> is set at a fixed 200px. When the carousel loaded the overthrow div is 700px. So the next arrow should be visible with about 1.5 tiles hidden. If I resize the browser the next arrow will become enabled. Please let me know if you need any additional info. Thanks
Matt

<div class="ectray-nextprev" tabindex="0">
<div class="sidescroll-nextprev-links">
<a href="#" class="sidescroll-prev disabled">Previous</a>
<a href="#" class="sidescroll-next disabled">Next</a>
</div><div class="ectraycarousel overthrow ectraysidescroll">
<ul id="ec-tiles" style="width: 1050px;">
<li class="article" data-ecid="urn:ngtv_enhanced_content:link-792"></li>
<li class="article" data-ecid="urn:ngtv_enhanced_content:link-793"></li>
<li class="video" data-ecid="urn:ngtv_enhanced_content:cnnvideo-794"></li>
<li class="video" data-ecid="urn:ngtv_enhanced_content:cnnvideo-795"></li>
<li class="video" data-ecid="urn:ngtv_enhanced_content:cnnvideo-796"></li>
</ul>
</div>
</div>
matthewbakerturner commented 11 years ago

Hi John,

I've got a bit of a tricky problem with asynchronous rendering of the side scroller items. I do know the total number items that will end up in the list, would it be possible to just pass in the item count and have the sidescroller use that instead of a DOM query to get the number?

Thanks Matt

[cid:image001.jpg@01CE20D4.132B3F40]http://bluetubeinc.com/

Matt Baker | Director of Software 1123 Zonolite Road, Suite 4 Atlanta, Georgia 30306 Tel: (678) 712-2115 Ex.305tel:678-712-2115,305 bluetubeinc.comhttp://bluetubeinc.com/

[cid:image002.png@01CE20D4.132B3F40] http://www.linkedin.com/company/bluetube-interactive [cid:image003.png@01CE20D4.132B3F40] https://twitter.com/bluetubei

From: John Bender notifications@github.com<mailto:notifications@github.com> Reply-To: filamentgroup/Overthrow reply@reply.github.com<mailto:reply@reply.github.com> Date: Thursday, September 12, 2013 7:54 PM To: filamentgroup/Overthrow Overthrow@noreply.github.com<mailto:Overthrow@noreply.github.com> Cc: matthewbakerturner matthew.baker2@turner.com<mailto:matthew.baker2@turner.com> Subject: Re: [Overthrow] Sidescroller nav auto hiding not always working (#51) Resent-From: Matthew.Baker2@turner.com<mailto:Matthew.Baker2@turner.com> Resent-Date: Thursday, September 12, 2013 7:54 PM

Matt,

Sorry for the response cycle here.

http://filamentgroup.github.io/Overthrow/examples/sidescroller/disabled-nav.html

That examples shows a sidescroller that needs scrolling. And the same for the fixed-width:

http://fixed-width.origin.overthrow-cnn.fgtest.com/examples/sidescroller/fixed-width.html

So I'll need some more detail on how to reproduce this. What browser are you using?

Reply to this email directly or view it on GitHubhttps://github.com/filamentgroup/Overthrow/issues/51#issuecomment-24364562.

johnbender commented 11 years ago

Matt,

As a workaround try doing the calculation yourself and setting the width. That is, move that .css call you had in there earlier AFTER the call to the sidescroller, and then call refresh on the sidescroller. Eg,

var calculatedWidth = /* ... future slide count * fixed slide width */ ;

overthrow.sidescroller( scroller, { /* ... options ... */ } );
$scroller.find( "ul" ).css( "width", calculatedWidth );
overthrow.sidescroller( scroller, "refresh" );
johnbender commented 11 years ago

Matt,

Generally speaking you can just use the refresh call to get things reset. The disabled nav plugin is listening for a refresh event.

johnbender commented 11 years ago

Matt,

Hmm, I think the refresh will have to come after all the items are reloaded otherwise it will break the width assignment. Disregard the above.

matthewbakerturner commented 11 years ago

Calling refresh after initializing the carousel actually helps. The items are all able to render before I call refresh and then refresh fixes the width. Did run into something else though. My items have a margin set but the internal calculation to figure out the width doesn't take it into account so the ul ends up not being wide enough. Any thoughts on that one?

Thanks Matt

[cid:image001.jpg@01CE20D4.132B3F40]http://bluetubeinc.com/

Matt Baker | Director of Software 1123 Zonolite Road, Suite 4 Atlanta, Georgia 30306 Tel: (678) 712-2115 Ex.305tel:678-712-2115,305 bluetubeinc.comhttp://bluetubeinc.com/

[cid:image002.png@01CE20D4.132B3F40] http://www.linkedin.com/company/bluetube-interactive [cid:image003.png@01CE20D4.132B3F40] https://twitter.com/bluetubei

From: John Bender notifications@github.com<mailto:notifications@github.com> Reply-To: filamentgroup/Overthrow reply@reply.github.com<mailto:reply@reply.github.com> Date: Friday, September 13, 2013 2:07 PM To: filamentgroup/Overthrow Overthrow@noreply.github.com<mailto:Overthrow@noreply.github.com> Cc: matthewbakerturner matthew.baker2@turner.com<mailto:matthew.baker2@turner.com> Subject: Re: [Overthrow] Sidescroller nav auto hiding not always working (#51) Resent-From: Matthew.Baker2@turner.com<mailto:Matthew.Baker2@turner.com> Resent-Date: Friday, September 13, 2013 2:13 PM

Matt,

Hmm, I think the refresh will have to come after all the items are reloaded otherwise it will break the width assignment. Disregard the above.

Reply to this email directly or view it on GitHubhttps://github.com/filamentgroup/Overthrow/issues/51#issuecomment-24413392.

johnbender commented 11 years ago

Hmm, we didn't account for that in our code though that's entirely reasonable. I would move the margin inside the LI to a wrapping div if you can. That would make the LI based width calculations accurate again.

On Fri, Sep 13, 2013 at 11:21 AM, matthewbakerturner < notifications@github.com> wrote:

Calling refresh after initializing the carousel actually helps. The items are all able to render before I call refresh and then refresh fixes the width. Did run into something else though. My items have a margin set but the internal calculation to figure out the width doesn't take it into account so the ul ends up not being wide enough. Any thoughts on that one?

Thanks Matt

[cid:image001.jpg@01CE20D4.132B3F40]http://bluetubeinc.com/

Matt Baker | Director of Software 1123 Zonolite Road, Suite 4 Atlanta, Georgia 30306 Tel: (678) 712-2115 Ex.305tel:678-712-2115,305 bluetubeinc.comhttp://bluetubeinc.com/

[cid:image002.png@01CE20D4.132B3F40] < http://www.linkedin.com/company/bluetube-interactive> [cid:image003.png@01CE20D4.132B3F40] https://twitter.com/bluetubei

From: John Bender <notifications@github.com<mailto: notifications@github.com>> Reply-To: filamentgroup/Overthrow <reply@reply.github.com<mailto: reply@reply.github.com>> Date: Friday, September 13, 2013 2:07 PM To: filamentgroup/Overthrow <Overthrow@noreply.github.com<mailto: Overthrow@noreply.github.com>> Cc: matthewbakerturner <matthew.baker2@turner.com<mailto: matthew.baker2@turner.com>> Subject: Re: [Overthrow] Sidescroller nav auto hiding not always working (#51) Resent-From: Matthew.Baker2@turner.com<mailto:Matthew.Baker2@turner.com>

Resent-Date: Friday, September 13, 2013 2:13 PM

Matt,

Hmm, I think the refresh will have to come after all the items are reloaded otherwise it will break the width assignment. Disregard the above.

Reply to this email directly or view it on GitHub< https://github.com/filamentgroup/Overthrow/issues/51#issuecomment-24413392>.

— Reply to this email directly or view it on GitHubhttps://github.com/filamentgroup/Overthrow/issues/51#issuecomment-24414309 .

matthewbakerturner commented 11 years ago

Unfortunately, there's a good bit a styling around the li and it's not really practical to create a wrapping div. Any other thoughts? I was able to manually set the ul width which mostly works. Also, not sure if it's related, with my manually calculated width, when the left arrow is disabled it's still clickable and actually acts like the right arrow.

Thanks Matt

[cid:image001.jpg@01CE20D4.132B3F40]http://bluetubeinc.com/

Matt Baker | Director of Software 1123 Zonolite Road, Suite 4 Atlanta, Georgia 30306 Tel: (678) 712-2115 Ex.305tel:678-712-2115,305 bluetubeinc.comhttp://bluetubeinc.com/

[cid:image002.png@01CE20D4.132B3F40] http://www.linkedin.com/company/bluetube-interactive [cid:image003.png@01CE20D4.132B3F40] https://twitter.com/bluetubei

From: John Bender notifications@github.com<mailto:notifications@github.com> Reply-To: filamentgroup/Overthrow reply@reply.github.com<mailto:reply@reply.github.com> Date: Friday, September 13, 2013 2:27 PM To: filamentgroup/Overthrow Overthrow@noreply.github.com<mailto:Overthrow@noreply.github.com> Cc: matthewbakerturner matthew.baker2@turner.com<mailto:matthew.baker2@turner.com> Subject: Re: [Overthrow] Sidescroller nav auto hiding not always working (#51) Resent-From: Matthew.Baker2@turner.com<mailto:Matthew.Baker2@turner.com> Resent-Date: Friday, September 13, 2013 2:28 PM

Hmm, we didn't account for that in our code though that's entirely reasonable. I would move the margin inside the LI to a wrapping div if you can. That would make the LI based width calculations accurate again.

On Fri, Sep 13, 2013 at 11:21 AM, matthewbakerturner < notifications@github.commailto:notifications@github.com> wrote:

Calling refresh after initializing the carousel actually helps. The items are all able to render before I call refresh and then refresh fixes the width. Did run into something else though. My items have a margin set but the internal calculation to figure out the width doesn't take it into account so the ul ends up not being wide enough. Any thoughts on that one?

Thanks Matt

[cid:image001.jpg@01CE20D4.132B3F40mailto:image001.jpg@01CE20D4.132B3F40]http://bluetubeinc.com/

Matt Baker | Director of Software 1123 Zonolite Road, Suite 4 Atlanta, Georgia 30306 Tel: (678) 712-2115 Ex.305tel:678-712-2115,305 bluetubeinc.comhttp://bluetubeinc.com/

[cid:image002.png@01CE20D4.132B3F40mailto:image002.png@01CE20D4.132B3F40] < http://www.linkedin.com/company/bluetube-interactive> [cid:image003.png@01CE20D4.132B3F40mailto:image003.png@01CE20D4.132B3F40] https://twitter.com/bluetubei

From: John Bender notifications@github.com<mailto:notifications@github.com<mailto: notifications@github.commailto:notifications@github.com>> Reply-To: filamentgroup/Overthrow reply@reply.github.com<mailto:reply@reply.github.com<mailto: reply@reply.github.commailto:reply@reply.github.com>> Date: Friday, September 13, 2013 2:07 PM To: filamentgroup/Overthrow Overthrow@noreply.github.com<mailto:Overthrow@noreply.github.com<mailto: Overthrow@noreply.github.commailto:Overthrow@noreply.github.com>> Cc: matthewbakerturner matthew.baker2@turner.com<mailto:matthew.baker2@turner.com<mailto: matthew.baker2@turner.commailto:matthew.baker2@turner.com>> Subject: Re: [Overthrow] Sidescroller nav auto hiding not always working (#51) Resent-From: Matthew.Baker2@turner.com<mailto:Matthew.Baker2@turner.commailto:Matthew.Baker2@turner.com>

Resent-Date: Friday, September 13, 2013 2:13 PM

Matt,

Hmm, I think the refresh will have to come after all the items are reloaded otherwise it will break the width assignment. Disregard the above.

Reply to this email directly or view it on GitHub< https://github.com/filamentgroup/Overthrow/issues/51#issuecomment-24413392>.

Reply to this email directly or view it on GitHubhttps://github.com/filamentgroup/Overthrow/issues/51#issuecomment-24414309 .

Reply to this email directly or view it on GitHubhttps://github.com/filamentgroup/Overthrow/issues/51#issuecomment-24414721.

johnbender commented 11 years ago

Did you see my email about the google hangout? If you can join maybe we can sort it out quickly

matthewbakerturner commented 11 years ago

Missed it, I'll hop on now

[cid:image001.jpg@01CE20D4.132B3F40]http://bluetubeinc.com/

Matt Baker | Director of Software 1123 Zonolite Road, Suite 4 Atlanta, Georgia 30306 Tel: (678) 712-2115 Ex.305tel:678-712-2115,305 bluetubeinc.comhttp://bluetubeinc.com/

[cid:image002.png@01CE20D4.132B3F40] http://www.linkedin.com/company/bluetube-interactive [cid:image003.png@01CE20D4.132B3F40] https://twitter.com/bluetubei

From: John Bender notifications@github.com<mailto:notifications@github.com> Reply-To: filamentgroup/Overthrow reply@reply.github.com<mailto:reply@reply.github.com> Date: Friday, September 13, 2013 3:07 PM To: filamentgroup/Overthrow Overthrow@noreply.github.com<mailto:Overthrow@noreply.github.com> Cc: matthewbakerturner matthew.baker2@turner.com<mailto:matthew.baker2@turner.com> Subject: Re: [Overthrow] Sidescroller nav auto hiding not always working (#51) Resent-From: Matthew.Baker2@turner.com<mailto:Matthew.Baker2@turner.com> Resent-Date: Friday, September 13, 2013 3:13 PM

Did you see my email about the google hangout? If you can join maybe we can sort it out quickly

Reply to this email directly or view it on GitHubhttps://github.com/filamentgroup/Overthrow/issues/51#issuecomment-24417421.