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

Colapse and Scrollspy incompatible #5122

Closed
gruber76 opened this issue Sep 13, 2012 · 7 comments
Closed

Colapse and Scrollspy incompatible #5122

gruber76 opened this issue Sep 13, 2012 · 7 comments
Labels

Comments

@gruber76
Copy link

You can recreate this bug on the Bootstrap Javascript page (http://twitter.github.com/bootstrap/javascript.html) in any browser I've tried, including at least Chrome (21.0.1180.89) and IE10.

  • Click "Carousel" in the left-hand-nav
  • Observe that the left-hand-nav highlights Carousel
  • Click "Collapse" in the left-hand-nav
  • Observe that the left-hand-nav highlights Collapse
  • Click "Collapsible Group Item adding o/ms gradient and unprefixed one #1 so that all groups are collapsed
  • Click "Carousel" in the left-hand-nav
  • Observe that the left-hand-nav incorrectly highlights "Collapse"
  • Scroll, and/or click around, and you'll find that Scrollspy has not updated the heights for anything below the collapsed section.
@mdo
Copy link
Member

mdo commented Sep 13, 2012

This is because we're not refreshing the scrollspy in our demos. Let's see if @fat wants to make that change to the docs, but I don't see it as a huge issue.

@gruber76
Copy link
Author

So, the appropriate way to handle this is to combine the Collapse show/hide events with the Scrollspy refresh method, like:

$('#myCollapsible').on('hidden', function () {
$('[data-spy="scroll"]').each(function () {
var $spy = $(this).scrollspy('refresh')
});
});

It's really not optional if you're using both features. Updating the documentation would be a good step (and I would be happy to pitch in with that if that's the direction decided) but I think it might make sense to include code in one or both JS modules to check if the other one is running and register this function to the event if it hasn't already been registered.

Now, maybe that's overkill if the collapse is happening outside of the scroll area, but in my opinion scrollspy is going to be used more for navigation (as in Bootstrap docs) than elsewhere so the times when they're both present but don't interact are going to be edge cases.

@andrewdeandrade
Copy link

Hey @gruber76,

Thanks for opening this issue! Unfortunately, it looks like it fails to pass the criteria neccessary for submitting to bootstrap. The following things are currently failing:

  • should include a jsfiddle/jsbin illustrating the problem if tagged with js but not a feature

For a full list of issue filing guidelines, please refer to the bootstrap issue filing guidelines.

thanks!

@gruber76
Copy link
Author

In case you're not a bot: since the JS may or may not be important enough to fix but the issue with the documentation is related but can be addressed without modifying the core app, should I:

  • open up a new docs-only ticket and attach a fiddle here
    Or
  • open up two new tickets, one for each?

@gruber76
Copy link
Author

gruber76 commented Oct 8, 2012

Hey @koenpunt,

Thank you for your note. Unfortunately it appears to fail the Turing Test for interactions with humans. Please feel free to re-read the request for assistance posted September 26th and respond appropriately.

@koenpunt
Copy link
Contributor

koenpunt commented Oct 9, 2012

Ow crap, my bad, did run make haunt to see what it does and at first it showed me nothing, but now I see; it commented on all the issues..

@fat
Copy link
Member

fat commented Feb 8, 2013

not going to make this change – it's not a big deal

@fat fat closed this as completed Feb 8, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants