-
Notifications
You must be signed in to change notification settings - Fork 7
<FileTabs /> #8
base: master
Are you sure you want to change the base?
<FileTabs /> #8
Conversation
render() { | ||
const { | ||
color, | ||
name, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add active
prop here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
this.props.active ? styles.tabContainerActive : styles.tabContainer, | ||
{ backgroundColor: color }, | ||
]}> | ||
<Text style={styles.text}>{name.toUpperCase()}</Text> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove forced toUpperCase
{Children.map(this.props.children.filter(c => c), child => ( | ||
child.props.id === this.state.selected ? | ||
cloneElement(child, { active: true, onPress: this.handleTabPress }) : | ||
cloneElement(child, { active: false, onPress: this.handleTabPress }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rewrite like:
{Children.map(children, child => cloneElement(child, {
active: child.props.id === this.state.selected,
onPress: this.handleTabPress
}))}
|
||
static Item = Item | ||
|
||
constructor(props) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use class properties for initial state instead of constructor:
class Example {
state = { value: 10 }
}
} | ||
|
||
const styles = StyleSheet.create({ | ||
tabContainer: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rename to container
alignItems: 'center', | ||
height: 40, | ||
}, | ||
tabContainerActive: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rename to active
onPress={this.handlePress}> | ||
<View | ||
style={[ | ||
active ? styles.tabContainerActive : styles.tabContainer, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move container styles logic to render body:
const containerStyles = [
styles.container,
{ backgroundColor: color },
active && styles.active
]
|
||
static defaultProps = { | ||
active: false, | ||
onPess: () => (console.log('onPess')), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove console.log
|
||
export default class Item extends Component { | ||
static propTypes = { | ||
id: PropTypes.string, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make required
id: PropTypes.string, | ||
active: PropTypes.bool, | ||
color: PropTypes.string, | ||
name: PropTypes.string, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same
active: PropTypes.bool, | ||
color: PropTypes.string, | ||
name: PropTypes.string, | ||
onPress: PropTypes.func, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same as with "active" - we don't pas "onPress" to every item. So, if we make it required and pass single onPress to entire component, we have warning.
import React, { Component, PropTypes } from 'react' | ||
import { View, Text, TouchableWithoutFeedback, StyleSheet } from 'react-native' | ||
|
||
export default class Item extends Component { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change name to ColoredTabsItem
render() { | ||
return ( | ||
<View style={styles.container}> | ||
<View style={styles.tabContainer}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we really need this View
wrapper?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can get rid it.
justifyContent: 'center', | ||
alignItems: 'center', | ||
}, | ||
tabContainer: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
change name to wrapper
Blocked by #4 |
@fAns1k please rebase |
Rename to FileTabs, make text font size larger and bold, add more examples. |
<ColoredTabs.Item id="1" name="Two" color="orange" /> | ||
<ColoredTabs.Item id="2" name="Three" color="chartreuse" /> | ||
<ColoredTabs.Item id="3" name="Four" color="dodgerblue" /> | ||
< /ColoredTabs> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove empty space
|
||
| Name | Desc | Type | Default | ||
| --- | --- | --- | --- | | ||
| `name` | Text of tab. | String | `undefined` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice if you rename this prop into title
like in other components // @itsmepetrov
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree
* Implemented Carousel component * Implemented Carousel tests * Updated tests for Carousel, some fixes in openExamplesFor method * Updated e2e tests * Updated e2e tests * Updated e2e tests * Updated e2e tests * Added Dash component * Added info about Carousel component n README.md * Updated info about Carousel component in README.md * Added info about Dash component in README.md * Updated Carousel.Item component * Updated unit tests * Updated openExamplesFor tests
# Conflicts: # __tests__/unit/__snapshots__/Storyshots.test.js.snap
0076b8b
to
00a6a34
Compare
| Name | Desc | Type | Default | ||
| --- | --- | --- | --- | | ||
| `selected` | id of selected tab. | String | `undefined` | ||
| `onPress` | Handle press on tab action. Receive id of selected tab as argument. | Function | `undefined` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this function has default value () => {}
| `color` | Color of tab | String | `undefined` | ||
| `id` | Tab id. This id will passed to `onPress` for identify action. | String | `undefined` | ||
| `active` | Show item as active | Boolean | `false` | ||
| `onPress` | Handle press action | Function | `undefined` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this function has default value () => {}
<ColoredTabs.Item id="2" name="Three" color="chartreuse" /> | ||
<ColoredTabs.Item id="3" name="Four" color="dodgerblue" /> | ||
< /ColoredTabs> | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
your example is not working, rewrite like this:
import React, { PureComponent } from 'react'
import { ColoredTabs } from 'tipsi-ui-kit'
class Example extends PureComponent {
handlePress = id => console.log(`Tab with id ${id} is clicked!`)
const Example = () => (
<ColoredTabs onPress={this.handlePress}>
<ColoredTabs.Item id="0" title="One" color="crimson" />
<ColoredTabs.Item id="1" title="Two" color="orange" />
<ColoredTabs.Item id="2" title="Three" color="chartreuse" />
<ColoredTabs.Item id="3" title="Four" color="dodgerblue" />
</ColoredTabs>
)
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also please check eslint-plugin in your editor. We use 2 spaces as tab
length
container: { backgroundColor: ThemeConstants.BLACK }, | ||
}) | ||
|
||
export default themeable('Label', baseStyles, { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FileTabs
, not Label
- when 'selected' does not defined - set first tab, as selected.
@@ -3,6 +3,52 @@ React Native Tipsi custom UI elements | |||
|
|||
## Components | |||
|
|||
### `<ColoredTabs />` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No description provided.
|
||
Customizable colored tabs. | ||
|
||
#### ColoredTabs Props |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FileTabs
| `onPress` | Handle press on tab action. Receive id of selected tab as argument. | Function | `undefined` | ||
|
||
|
||
#### ColoredTabs.Item Props |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FileTabs
|
||
```js | ||
import React from 'react' | ||
import { ColoredTabs } from 'tipsi-ui-kit' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FileTabs
handlePress = (id) => (console.log(`Tab with id ${id} is clicked!`) | ||
|
||
const Example = () => ( | ||
<ColoredTabs onPress={this.handlePress}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FileTabs
|
||
const Example = () => ( | ||
<ColoredTabs onPress={this.handlePress}> | ||
<ColoredTabs.Item id="0" name="One" color="crimson" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FileTabs
|
||
```js | ||
import React, { PureComponent } from 'react' | ||
import { ColoredTabs } from 'tipsi-ui-kit' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FileTabs
<ColoredTabs.Item id="1" title="Two" color="orange" /> | ||
<ColoredTabs.Item id="2" title="Three" color="chartreuse" /> | ||
<ColoredTabs.Item id="3" title="Four" color="dodgerblue" /> | ||
</ColoredTabs> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FileTabs
Please rebase |
Also, don't forget check my messages too :) |
No description provided.