[Tabs] Use flat buttons for tabs #3051

onumossn · 2016-01-26T00:40:09Z

Use flat buttons for tab buttons to more closely comply with material design specs.

All caps text
Add ripples
Unfocused tab color: #fff 70%

oliviertassinari · 2016-01-26T09:32:50Z

@onumossn That will make the Tabs follow more closely the material specification 👍.

oliviertassinari · 2016-01-26T09:33:15Z

src/tabs/tab.jsx

-      <div
-        {...other}
-        style={this.prepareStyles(styles)}
+      <FlatButton


Can we use a <EnhancedButton /> instead?

mbrookes · 2016-01-26T10:29:29Z

@oliviertassinari How is this going to accommodate the changes being introduced in #3042?

oliviertassinari · 2016-01-26T10:41:54Z

Well, we will have a merge conflict on this PR.

mbrookes · 2016-01-26T10:46:26Z

Not just a merge conflict - FlatButton / EnhancedButton don't support the 'icon over text' layout.

oliviertassinari · 2016-01-26T10:53:55Z

Not just a merge conflict - FlatButton / EnhancedButton don't support the 'icon over text' layout.

I wasn't aware of this issue.

onumossn · 2016-01-26T18:43:28Z

Updated it to use EnhancedButton, and tried adding the code from #3042 to see if it would have any negative impacts, and it seemed to work fine. However, there may be minor merge conflicts, but they can be easily resolved. Also, I moved the transform(0,0,0) for ripples to work correctly from the specific implementations of buttons to the EnhancedButton.

I don't understand the lack of support. If it is used as container component like the div that was there, the children can be positioned and behave however they like within the container, so 'icon over text' should be work fine as long as both the icon and text are children.

mbrookes · 2016-01-26T19:10:22Z

@onumossn - yes, sorry, you are correct. Thanks for testing.

oliviertassinari · 2016-01-27T09:41:04Z

@onumossn We have just merged #3042. Could you rebase?

onumossn · 2016-01-28T03:48:41Z

Rebased

alitaheri · 2016-01-28T07:51:40Z

src/enhanced-button.jsx

@@ -285,6 +285,13 @@ const EnhancedButton = React.createClass({
      cursor: disabled ? 'default' : 'pointer',
      textDecoration: 'none',
      outline: 'none',
+      /*
+        This is need so that ripples do not bleed


alitaheri · 2016-01-28T07:55:21Z

Other than that it's great 👍 👍 Thanks a lot 😁

oliviertassinari · 2016-01-28T10:47:27Z

src/tabs/tab.jsx

      paddingTop: (icon && !label) ? 4 : 0,
      height: (label && icon) ? 72 : 48,
      color: textColor,
-      outline: 'none',
-      fontSize: 14,


Can we add it back?

oliviertassinari · 2016-01-28T10:49:05Z

I agree, I like this PR 😍.

onumossn · 2016-01-30T05:29:47Z

Does this look ok? I updated padding to have left and right padding, and set top and bottom padding to 0. It doesn't seem the conditional top padding is necessary with enhanced buttons. Let me know if you feel that it is necessary and I will put it back in.

alitaheri · 2016-01-30T06:20:39Z

Thanks a lot @onumossn Me and @oliviertassinari Will take a last look. Awesome work 👍 👍

alitaheri · 2016-01-30T06:53:24Z

👍 here. @oliviertassinari merge if it's alright.

oliviertassinari · 2016-01-30T10:22:22Z

@onumossn Thanks! Can you squash down the commit? I think that we can merge then 👍.

oliviertassinari · 2016-01-30T10:23:52Z

src/flat-button.jsx

@@ -266,13 +266,10 @@ const FlatButton = React.createClass({
      position: 'relative',
      overflow: 'hidden',
      backgroundColor: hovered ? buttonHoverColor : buttonBackgroundColor,
-      lineHeight: buttonHeight + 'px',
+      height: buttonHeight,


Since we are no longer using the FlatButton. I think that we can remove this change.

onumossn · 2016-02-01T04:41:53Z

Updated and rebased. Why is line-height used instead of height anyway?

newoga · 2016-02-01T05:23:01Z

Updated and rebased. Why is line-height used instead of height anyway?

I think since that <FlatButton /> just has text, the lineHeight is used to make sure it's vertically centered properly.

newoga · 2016-02-01T05:23:35Z

@oliviertassinari @alitaheri Seems like you both okayed this so I'll merge it.

[Tabs] Use flat buttons for tabs

newoga · 2016-02-01T05:26:13Z

Thanks much @onumossn!

maartenbusstra · 2016-02-05T07:01:01Z

Can I tap into the FlatButton, and disable the ripple somehow? This kind of screwed my navigation bar.

alitaheri · 2016-02-05T09:00:17Z

Yes we should improve the customization for this. I also broke my close button since buttons cannot nest buttons. I'm trying to figure out the best way to achieve this.

onumossn force-pushed the rippled-tabs branch from b24ea42 to 1fd1d86 Compare January 26, 2016 00:42

mbrookes added the PR: Needs Review label Jan 26, 2016

mbrookes changed the title ~~Use flat buttons for tabs~~ [Tabs] Use flat buttons for tabs Jan 26, 2016

oliviertassinari added the design: material This is about Material Design, please involve a visual or UX designer in the process label Jan 26, 2016

oliviertassinari reviewed Jan 26, 2016
View reviewed changes

src/tabs/tab.jsx

<div

{...other}

style={this.prepareStyles(styles)}

<FlatButton

Copy link

Member

oliviertassinari Jan 26, 2016

Choose a reason for hiding this comment

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

Can we use a <EnhancedButton /> instead?

oliviertassinari added PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI and removed PR: Needs Review labels Jan 26, 2016

onumossn force-pushed the rippled-tabs branch 2 times, most recently from 6deed43 to a3cede4 Compare January 28, 2016 03:44

alitaheri added PR: Needs Review and removed PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI labels Jan 28, 2016

alitaheri reviewed Jan 28, 2016
View reviewed changes

oliviertassinari reviewed Jan 28, 2016
View reviewed changes

oliviertassinari added PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI and removed PR: Needs Review labels Jan 28, 2016

alitaheri added the PR: Needs Review label Jan 30, 2016

alitaheri removed the PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI label Jan 30, 2016

oliviertassinari added PR: Review Accepted and removed PR: Needs Review labels Jan 30, 2016

oliviertassinari reviewed Jan 30, 2016
View reviewed changes

oliviertassinari added PR: Needs Review PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI and removed PR: Review Accepted labels Jan 30, 2016

Use enhanced buttons for tabs

5d848a8

onumossn force-pushed the rippled-tabs branch from 36f1249 to 5d848a8 Compare February 1, 2016 04:41

newoga added PR: Needs Review and removed PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI labels Feb 1, 2016

newoga added PR: Review Accepted and removed PR: Needs Review labels Feb 1, 2016

newoga added a commit that referenced this pull request Feb 1, 2016

Merge pull request #3051 from onumossn/rippled-tabs

5f47b3d

[Tabs] Use flat buttons for tabs

newoga merged commit 5f47b3d into mui:master Feb 1, 2016

This was referenced Sep 26, 2021

[Snyk] Upgrade eslint-plugin-react from 7.12.4 to 7.25.1 akashmitra/material-ui#3

Open

[Snyk] Upgrade eslint-plugin-react from 7.12.4 to 7.25.1 zhaleTaheri/material-ui#3

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Tabs] Use flat buttons for tabs #3051

[Tabs] Use flat buttons for tabs #3051

onumossn commented Jan 26, 2016

oliviertassinari commented Jan 26, 2016

oliviertassinari Jan 26, 2016

mbrookes commented Jan 26, 2016

oliviertassinari commented Jan 26, 2016

mbrookes commented Jan 26, 2016

oliviertassinari commented Jan 26, 2016

onumossn commented Jan 26, 2016

mbrookes commented Jan 26, 2016

oliviertassinari commented Jan 27, 2016

onumossn commented Jan 28, 2016

alitaheri Jan 28, 2016

alitaheri commented Jan 28, 2016

oliviertassinari Jan 28, 2016

oliviertassinari commented Jan 28, 2016

onumossn commented Jan 30, 2016

alitaheri commented Jan 30, 2016

alitaheri commented Jan 30, 2016

oliviertassinari commented Jan 30, 2016

oliviertassinari Jan 30, 2016

onumossn commented Feb 1, 2016

newoga commented Feb 1, 2016

newoga commented Feb 1, 2016

newoga commented Feb 1, 2016

maartenbusstra commented Feb 5, 2016

alitaheri commented Feb 5, 2016

[Tabs] Use flat buttons for tabs #3051

[Tabs] Use flat buttons for tabs #3051

Conversation

onumossn commented Jan 26, 2016

oliviertassinari commented Jan 26, 2016

oliviertassinari Jan 26, 2016

Choose a reason for hiding this comment

mbrookes commented Jan 26, 2016

oliviertassinari commented Jan 26, 2016

mbrookes commented Jan 26, 2016

oliviertassinari commented Jan 26, 2016

onumossn commented Jan 26, 2016

mbrookes commented Jan 26, 2016

oliviertassinari commented Jan 27, 2016

onumossn commented Jan 28, 2016

alitaheri Jan 28, 2016

Choose a reason for hiding this comment

alitaheri commented Jan 28, 2016

oliviertassinari Jan 28, 2016

Choose a reason for hiding this comment

oliviertassinari commented Jan 28, 2016

onumossn commented Jan 30, 2016

alitaheri commented Jan 30, 2016

alitaheri commented Jan 30, 2016

oliviertassinari commented Jan 30, 2016

oliviertassinari Jan 30, 2016

Choose a reason for hiding this comment

onumossn commented Feb 1, 2016

newoga commented Feb 1, 2016

newoga commented Feb 1, 2016

newoga commented Feb 1, 2016

maartenbusstra commented Feb 5, 2016

alitaheri commented Feb 5, 2016