-
-
Notifications
You must be signed in to change notification settings - Fork 32.4k
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
[Tabs] Add icon and text #3042
[Tabs] Add icon and text #3042
Conversation
import FontIcon from 'material-ui/lib/font-icon'; | ||
import ActionFlightTakeoff from 'material-ui/lib/svg-icons/action/flight-takeoff'; | ||
|
||
const styles = { |
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.
To remove
Shouldn't we put the icon above the label? |
Wow how can I miss that ? Fixed ;) |
import FontIcon from 'material-ui/lib/font-icon'; | ||
import ActionFlightTakeoff from 'material-ui/lib/svg-icons/action/flight-takeoff'; | ||
|
||
const TabsExampleSimple = () => ( |
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.
TabsExampleIcon
?
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.
Fixed
@@ -19,13 +23,15 @@ const descriptions = { | |||
swipeable: 'This example integrates the [react-swipeable-views]' + | |||
'(https://github.com/oliviertassinari/react-swipeable-views) component with Tabs, animating the Tab transition, ' + | |||
'and allowing tabs to be swiped on touch devices.', | |||
icon: 'An example of tabs with icon', | |||
iconText: 'An example of tabs with icon and 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.
Could you put a full-stop at the end of each sentence?
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
@pradel That looks good 👍. Do you want to try a squash down of the commits 😁? Thanks. |
Okay squashed this time working :) |
@pradel Awesome. It's almost good. Can you add a bit of spacing between the label and the icon to follow more closely the specification? Like 2 or 4 pixels. |
@oliviertassinari like that is better ? |
@pradel That's much better 👍! Thanks. Can you squash down again the commits 😬? |
- Added tab with icon - Added tab with icon and text - Added one example for each Support `FontIcon`and `SvgIcon`. remove unused styles for eslint inverse label and icon position changed let to const Fix const name example Put Example to lowercase Selected color to const Add react.isValidElement check Add full-stop Added 4px between icon and text
Squashed :) |
[Tabs] Add icon and text
@pradel Thanks! |
Support
FontIcon
andSvgIcon
.