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

[Tab] Change the ripple color to follow the spec #3857

Merged
merged 1 commit into from
Apr 2, 2016
Merged

[Tab] Change the ripple color to follow the spec #3857

merged 1 commit into from
Apr 2, 2016

Conversation

oliviertassinari
Copy link
Member

The spec is at the very bottom of this page.

@oliviertassinari oliviertassinari added PR: Needs Review design: material This is about Material Design, please involve a visual or UX designer in the process labels Apr 2, 2016
@@ -117,8 +119,7 @@ class Tab extends React.Component {
iconElement = React.cloneElement(icon, params);
}

const rippleColor = styles.color;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's undefined.

@mbrookes
Copy link
Member

mbrookes commented Apr 2, 2016

Noticed you're using all-caps for the const here, but (AFAIK?) none of the other consts in MUI are.

If we're going to change the code style, shouldn't we do so consistently across library, and supported by an eslint rule?

@oliviertassinari
Copy link
Member Author

@mbrookes I have removed the change regarding the const. I was going toward this rule http://eslint.org/docs/rules/no-magic-numbers. But that's out of the scope of this PR.

@mbrookes
Copy link
Member

mbrookes commented Apr 2, 2016

Looks good! 👍

@mbrookes mbrookes merged commit dc4c74d into mui:master Apr 2, 2016
@oliviertassinari oliviertassinari deleted the tabs-fix-ripple branch November 13, 2016 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design: material This is about Material Design, please involve a visual or UX designer in the process
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants