Improve styling of flat buttons

[troutowicz]

• hoverColor prop
• rippleColor prop
• persist hover state through ripple
• miscellaneous refinements

If this gets a thumbs up, I will do the same for raised buttons.

[pomerantsev]

@troutowicz I did some research, and I have feedback on the PR. Hope it will be valuable to you.
The code has become somewhat brittle, because in _handleKeyboardFocus and _handleTouchTap we manipulate styles directly, and the whole purpose of _handleTouchTap is to cancel the change that has been done in _handleKeyboardFocus (we're relying on the fact that _handleKeyboardFocus fires first).
What I propose is getting rid of _handleTouchTap altogether and instead just using state to indicate whether a button is focused with a keyboard. Then in styles we would apply rootWhenHovered only when this.state.hovered || this.state.isKeyboardFocused.
So the style for enhancedButton would look like (you could still make it less repetitive, I guess):

style={this.mergeAndPrefix(
                               styles.root,
                               ((this.state.hovered || this.state.isKeyboardFocused) && !this.props.disabled) && styles.rootWhenHovered,
                               this.props.style,
                               ((this.state.hovered || this.state.isKeyboardFocused) && !this.props.disabled) && this.props.hoverStyle
                             )}

And these will be the event handlers:

  _handleMouseOver (e) {
    this.setState({hovered: true});
    if (this.props.onMouseOver) {
      this.props.onMouseOver(e);
    }
  },

  _handleMouseOut (e) {
    this.setState({hovered: false});
    if (this.props.onMouseOut) {
      this.props.onMouseOut(e);
    }
  },

  _handleKeyboardFocus (e, isKeyboardFocused) {
    this.setState({isKeyboardFocused});
  }

Notice that we no longer check if the enhanced button is focused with a keyboard since we always want the hovered state to be applied when the mouse is actually over the button.

I would also add an onBlur handler that would as well set hovered state to false - this way we cover all bases on mobile as well, since onMouseOver fires there, but not onMouseOut, so a button can stay in a hovered state after we press another button.

[troutowicz]

@pomerantsev Thanks for the critique. Your suggestions make sense, I will make the changes later today... I wasn't happy with the handleTouchTap hack and the handleKeyboardFocus didn't sit well with me either.

Also, I recently updated this PR to instead have hoverColor and rippleColor props instead of a hoverStyle prop.

[pomerantsev]

Great.

[troutowicz]

@pomerantsev Had some free time on lunch, I have updated the PR.

[pomerantsev]

@troutowicz Thanks. Tested it quickly, looks good.
This project would benefit greatly from a test suite - I wonder how guys make sure nothing's broken with every new PR.

[troutowicz]

The project definitely needs a test suite. I personally have been testing my changes inside a personal project that uses the various components that I submit a PR for.

Side note: My project uses mochify, and has worked well for my component tests thus far. (my project's tests)

[pomerantsev]

Same with me - I've also been testing stuff inside a different project.

[hai-cea]

@troutowicz @pomerantsev There is an issue created for adding a test suite #396 - maybe we can have a discussion there and come up with a decision on which test suite to use?

[hai-cea]

Looks good - Thanks @troutowicz & @pomerantsev 👍