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 to support nested carousels #8449

Closed
wants to merge 1 commit into from
Closed

Conversation

Deele
Copy link

@Deele Deele commented Jul 11, 2013

To allow usage of nested carousels, there is a need to constraint selectors to only first level of elements so children carousels will be left untouched by sliding of parent carousel.

Related issues:
#4359 unable to put carousel inside carousel
#7734 Conflict on item divs using nested carousel

Fix tested and working on:

  • Mozilla Firefox 22.0
  • Mozilla Firefox Aurora 24.0a2
  • Google Chrome 26.0.1410.64 m

PS: Sorry if I format anything in wrong way, this is my second pull request (first was failure #8448).

To allow usage of nested carousels, there is a need to constraint selectors to only first level of elements so children carousels will be left untouched by sliding of parent carousel.

Related issues:
twbs#4359 unable to put carousel inside carousel
twbs#7734 Conflict on item divs using nested carousel

PS: Sorry if I format anything in wrong way, this is my second pull request (first was failure twbs#8448).
@mdo
Copy link
Member

mdo commented Jul 11, 2013

There is absolutely no way we're supporting nested carousels.

@mdo mdo closed this Jul 11, 2013
@Deele
Copy link
Author

Deele commented Jul 11, 2013

@mdo Could you please provide more detail in reasoning behind your decision
@cvrebert As you can see, in my description I already mentioned your given issue ID. No need to repeat.

@mdo
Copy link
Member

mdo commented Jul 11, 2013

In short, @Deele, it sounds like an awful experience and I see it going bad more often than it going well.

@Deele
Copy link
Author

Deele commented Jul 16, 2013

@mdo Could you give us "the long" answer? Because I can't see any document, style guide or any other documented reasoning, that this is awful experience. From my point of view, designers of pages should think about "good experience", and let them do mistakes for themselves. Bootstrap is a tool, and same as HTML allows to nest anything in any tag, bootstrap should allow the same, otherwise it is silly to make such a constraints. This should be universal tool.

@goto-bus-stop
Copy link

...Is there any use case at all for nested carousels?

@mdo
Copy link
Member

mdo commented Jul 16, 2013

What @renekooi said. I can't think of a single good experience that could be had from nesting carousels.

From my point of view, designers of pages should think about "good experience", and let them do mistakes for themselves.

Not a bad philosophy, but some mistakes can be prevented altogether 😄.

Bootstrap is a tool, and same as HTML allows to nest anything in any tag, bootstrap should allow the same, otherwise it is silly to make such a constraints.

Yes, not quite, probably not, and also not quite:

  1. Yes, it is a tool—one that we've provided and maintained for quite some time to help folks get stuff done at a decent level of quality.
  2. HTML doesn't allow you to nest anything in anything. I'm sure I'm being overly pedantic, but I want to clarify that's not the case (anchors in anchors, divs in inputs, etc). And that leads into the next point...
  3. Just because Bootstrap uses HTML and CSS doesn't mean it is bound to any preconceptions about those languages. We do what's best for developers and designers to build stuff well.
  4. And lastly, it's not silly to have constraints. Working within constraints is part of good design and development, and we're going to enforce such constraints as well as we can to help avoid bad experiences for your own users.

Hope that helps <3.

@ludoblues
Copy link

A very simple use case in 2 lines:

A carousel composed by groups (each item is a group and we have n "group" entities).
On each group item, a carousel of its members (each item is a bunch of members and we have n "members" entities).

Is it a crazy idea ? What is "bad" here ? And how could we compare that as a div in an input ??

This use case is more and more required by the mobile devices experiences.

@mdo
Copy link
Member

mdo commented Jul 25, 2013

If you can design and prototype a quality nested carousel, we'll reconsider. Until then, I'm still not convinced it's ever a good idea. I also don't see how this would be required by mobile devices.

@gribelu
Copy link
Contributor

gribelu commented Aug 23, 2013

I find that this feature can come in handy when one needs to slide html content (some sort of pagination) and that content also has an image carousel inside.

The project I'm currently working on needed nested carousels to paginate article/project lists. Also, each article/project can have a carousel on it's thumbnail.
I patched the Bootstrap 3 carousel with code very similar to this pull request and it works just fine.

This is an example of what was needed:
screenshot9

@cavico
Copy link

cavico commented Jan 27, 2014

@Deele its a clean and great solution to my carousel problem.

Thanks!

@Deele
Copy link
Author

Deele commented Jan 27, 2014

@gribelu Making that one responsive and usable from UX point of view, would be a challenge.
@Mick3D This is here, to make folks like you, get less headaches :)
@shuhad "Lots of nested carousel" very bad. I would suggest no more than another level of carousel inside main one.
@JakeStoeffler They have their own design principle, if ignored once, could lead to cascading problems in with other widgets. I understand their view.
@cavico You are welcome.

@ajdvoynos
Copy link

Huge time saver, thanks for this @Deele. Would have loved to see it get merged @mdo , I have a perfectly valid use case as well.

@twbs twbs locked and limited conversation to collaborators Jun 10, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants