Update Carousel #3201
Update Carousel #3201
Conversation
|
this is done i think |
src/Carousel.js
Outdated
| import * as ValidComponentChildren from './utils/ValidComponentChildren'; | ||
| import { createBootstrapComponent } from './ThemeProvider'; | ||
|
|
||
| // TODO: `slide` should be `animate`. |
| prevIcon: <Glyphicon glyph="chevron-left" />, | ||
| activeIndex: 0, | ||
|
|
||
| prevIcon: <span className="carousel-control-prev-icon" aria-hidden="true" />, |
There was a problem hiding this comment.
should this derive from bsClass somehow?
www/src/html.js
Outdated
| @@ -1,46 +0,0 @@ | |||
| import PropTypes from 'prop-types'; | |||
| @@ -11,7 +11,6 @@ class ControlledCarousel extends React.Component { | |||
| } | |||
|
|
|||
| handleSelect(selectedIndex, e) { | |||
| alert(`selected=${selectedIndex}, direction=${e.direction}`); | |||
| @@ -0,0 +1,5 @@ | |||
| // reading a dimension prop will cause the browser to recalculate, | |||
|
|
||
| setPostBodyComponents([ | ||
| <script | ||
| key="asfasfasfas" |
| } | ||
| let direction; | ||
| if ( | ||
| (nextIndex === 0 && previousActiveIndex >= lastPossibleIndex) || |
There was a problem hiding this comment.
when does this trigger? if previously there were 3 items, but now there's only 1? why should that trigger "next", v going from 3 -> 2 trigger "prev"?
There was a problem hiding this comment.
No, when the slides wrap you want to continue in the same direction you we're going. this isn't a perfect check but I think this component is dumb and don't want to spend lots of time on it :P
There was a problem hiding this comment.
hmm so that's if previousActiveIndex == lastPossibleIndex... except what if the user clicks on the indicator? or if you're cycling back from 0 to final
| const { slide, interval, activeIndex: activeIndexProp } = this.props; | ||
| select(index, event, direction) { | ||
| clearTimeout(this.selectThrottle); | ||
| if (event && event.persist) event.persist(); |
There was a problem hiding this comment.
we can probably kill this logic of decorating the event
| }; | ||
|
|
||
| handleMouseOut = () => { | ||
| this.cycle(); |
There was a problem hiding this comment.
only if we're doing "pause on hover"? otherwise why should mousing out reset the timer?
| const { | ||
| activeIndex, | ||
| previousActiveIndex, | ||
| prevClasses, |
There was a problem hiding this comment.
maybe prevClassName and currentClassName would be better
this is such a meh component…
still doesn’t have keyboard events or support for the other animations…