[SpeedDial] Change actions background color #14640

hburrows · 2019-02-23T16:03:40Z

The SpeedDial component uses the Fab component but provides no way to control or override the color of the embedded Fab component. This PR continues to default the Fab color to primary but does so by utilizing ButtonProps in a manner that allows the caller to provide an override (for example, to use default or secondary color).
This PR also changes the default SpeedDialAction button color from grey to white whic better aligns with the spec (or at least the examples in the spec).

I have followed (at least) the PR section of the contributing guide.

mui-pr-bot · 2019-02-23T16:07:43Z

Details of bundle changes.

Comparing: 7a49f80...a19c618

bundle	parsed diff	gzip diff	prev parsed	current parsed	prev gzip	current gzip
@material-ui/core	0.00%	0.00%	371,832	371,832	91,939	91,939
@material-ui/core/Paper	0.00%	0.00%	76,648	76,648	19,308	19,308
@material-ui/core/Paper.esm	0.00%	0.00%	71,599	71,599	18,783	18,783
@material-ui/core/Popper	0.00%	0.00%	30,462	30,462	10,582	10,582
@material-ui/core/styles/createMuiTheme	0.00%	0.00%	17,286	17,286	5,717	5,717
@material-ui/core/useMediaQuery	0.00%	0.00%	2,486	2,486	1,049	1,049
@material-ui/lab	-0.02%	-0.01%	184,281	184,244	50,584	50,581
@material-ui/styles	0.00%	0.00%	55,687	55,687	15,825	15,825
@material-ui/system	0.00%	0.00%	17,062	17,062	4,482	4,482
Button	0.00%	0.00%	99,409	99,409	26,607	26,607
Modal	0.00%	0.00%	98,984	98,984	26,263	26,263
colorManipulator	0.00%	0.00%	3,232	3,232	1,296	1,296
docs.landing	0.00%	0.00%	51,891	51,891	11,291	11,291
docs.main	0.00%	0.00%	678,326	678,326	206,184	206,184
packages/material-ui/build/umd/material-ui.production.min.js	0.00%	0.00%	322,466	322,466	85,044	85,044

Generated by 🚫 dangerJS against a19c618

oliviertassinari · 2019-02-23T18:34:24Z

@hburrows Hi, we are looking for somebody to take the ownership of this component. Would you be interested in doing so? We were considering the option to drop the support of the component in #12830 (comment), but keeping it in the lab and eventualy move it to the core is a viable option if we have somebody from the community than can take care of it. Alternativelty, we can move it to somewhere else. cc @mbrookes.

mbrookes · 2019-02-23T21:34:09Z

Semi related - looking at the spec, it seems the default color scheme for the FAB is a white icon on a black background, with an option for grey on white, which the speed dial actions use by default. Not sure how that would fit into our current color prop options.

Yes, we'd be gad to have someone take primary ownership of the SpeedDial component - review PRs etc...

hburrows · 2019-02-24T16:20:04Z

@oliviertassinari @mbrookes I'm tentatively open to helping with the component and shepherding it along into core (eventually). My major hesitation is that I'm not committed to using the component yet (vs. a more traditional popup menu). I like the idea of the speed-dial but my concern is that it might be too confusing for some of my product's users vs a more tradition popup menu opened via the Fab - maybe what I want is the Fab transforming into a menu as defined in the spec. But for now, I'm open to reviewing PRs for the component and doing necessary maintenance/evolution. I use this project a lot so it's only fair that I contribute back 😄

mbrookes · 2019-02-24T16:28:29Z

Great! Thanks @hburrows.

Any thoughts on the MDv2 spec FAB / SpeedDial colors?

hburrows · 2019-02-24T16:35:42Z

@mbrookes Can you please share a link to the Material Design V2 specs that you're looking at so I know I'm looking at the same thing you're looking at. What I'm looking at doesn't reference any version number.

mbrookes · 2019-02-24T16:42:39Z

Sorry, the current spec. We loosely refer to it as v2 after Google gave it an overhaul early last year.

https://material.io/design/components/buttons-floating-action-button.html

hburrows · 2019-02-27T16:22:42Z

@mbrookes Thanks for confirming the link to the spec -- we're looking at the same thing. The spec is rather vague regarding the exact color scheme, but in the Anatomy section states "The FAB is typically displayed in a circular container. An app’s color scheme determines its color fill, which should contrast with the area behind the FAB." Many of the examples use a FAB with a black fill and a white icon, but just as many use a different color aligned with the theme of the example. I feel that the behavior of the SpeedDial component as currently implemented is correct -- it uses the "primary" color theme for the FAB and (per my PR) allows for the "color" property to be overridden. As for the SpeedDialAction component it's less clear what the default color should be. The examples in the spec show white fill with grey icons... but I'm not seeing any specific language regarding the color of the action buttons. I would personally favor white fill with grey icon as the default, as it seems a little less opinionated, with the ability to very easily control the colors. I think the best approach is in the spirit of the spec, aligned with other material-ui component behavior, and the ability to easily customize.

mbrookes · 2019-02-27T18:45:09Z

Okay, sounds good. Would you like to update the SpeedDialAction default color & customizability as part of this PR, or treat it separately?

mbrookes · 2019-02-27T18:48:50Z

packages/material-ui-lab/src/SpeedDial/SpeedDial.js

@@ -303,6 +304,11 @@ SpeedDial.propTypes = {
   * @ignore


Just noticed that some of the props above refer to the Button component, whereas the Fab component was extracted out of the Button. Do you want to fix those up?

@mbrookes I'll address the SpeedDialAction color as part of this PR -- I think that makes sense. For expediency, I'm good with fixing the props as part of the PR also.

So... changing from ButtonProps to FabProps is a breaking change... but so is changing the default color for the action buttons but won't break code. Any ground rules for how breaking changes impact "Lab" components? I'm now thinking maybe it's best to change the prop name (ButtonProps -> FabProps) in a separate PR

Technically the only difference is how we change the version number. However, we are a bit more liberal with breaking changes in the lab since the API isn't considered stable.

I'm joining, what's the reasoning against?

<SpeedDial FabProps={{ color: 'secondary' }} />

It looks simple to me.

No reasoning against that... it just didn't occur to me to express it that way. I guess I was kind of thinking the SpeedDial "is a" special type of Fab instead of "has a" Fab (if that makes any sense)... so I expressed it the same way you would with a Fab (as a direct prop). We're already using FabProps (or will be in a subsequent PR) so I'm on board with removing the direct color prop and moving it to ButtonProps

Should I just change ButtonProps to FabProps as part of this PR instead of a separate PR?

…closer with spec

…fab-color

…essing Fab color

…fab-color

hburrows · 2019-03-05T22:11:51Z

@oliviertassinari @mbrookes Can either of you help me interpret the Argos-CI error. I'm not familiar with it -- but I suspect changing the default SpeedDialAction background color from grey to white might be the issue.

hburrows · 2019-03-07T15:36:33Z

@oliviertassinari Thanks for removing that redundant code. I got too wrapped around the axle of this trivial change to actually see that.

rahuliet · 2019-06-18T07:27:37Z

ButtonProps={{ size:'small', style: {"background":"green"}}}

for me wrapping background in style attribute of button worked for me

Add color property for passing to Fab component

0740f1b

Fix eslint line length issue

ab1a69b

hburrows changed the title ~~Add color property for passing to Fab component~~ [SpeedDial] Add color property for passing to Fab component Feb 23, 2019

Updating documentation markdown; updating typescript declaration

2f4e629

mbrookes added the component: speed dial This is the name of the generic UI component, not the React module! label Feb 23, 2019

mbrookes reviewed Feb 27, 2019

View reviewed changes

hburrows added 8 commits March 4, 2019 07:53

Changing default color of action buttons from grey to white to align …

9dfd59b

…closer with spec

Merge branch 'next' of github.com:mui-org/material-ui into speeddial-…

8145ee9

…fab-color

Merge branch 'next' of github.com:mui-org/material-ui into speeddial-…

a447055

…fab-color

Reverting dedicated color prop in favor of using ButtonProps for expr…

0e90605

…essing Fab color

Updating doc after reverting dedicated color prop

5612cc0

Merge branch 'next' of github.com:mui-org/material-ui into speeddial-…

d399d05

…fab-color

Reverting mistakenly including PR mui#14641

1529dfb

Adding back default Fab color styling of 'primary'

6248887

hburrows and others added 2 commits March 6, 2019 13:18

Changing how Fab color is faulted; previous impl. doesn't work correclty

286d637

Remove redundant code

a19c618

oliviertassinari changed the title ~~[SpeedDial] Add color property for passing to Fab component~~ [SpeedDial] Change actions background color Mar 6, 2019

oliviertassinari approved these changes Mar 6, 2019

View reviewed changes

oliviertassinari added the PR: good for merge label Mar 6, 2019

oliviertassinari merged commit 622066c into mui:next Mar 7, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SpeedDial] Change actions background color #14640

[SpeedDial] Change actions background color #14640

hburrows commented Feb 23, 2019 •

edited by oliviertassinari

Loading

mui-pr-bot commented Feb 23, 2019 •

edited

Loading

oliviertassinari commented Feb 23, 2019

mbrookes commented Feb 23, 2019 •

edited

Loading

hburrows commented Feb 24, 2019

mbrookes commented Feb 24, 2019

hburrows commented Feb 24, 2019

mbrookes commented Feb 24, 2019

hburrows commented Feb 27, 2019

mbrookes commented Feb 27, 2019

mbrookes Feb 27, 2019

hburrows Feb 27, 2019

hburrows Feb 27, 2019

eps1lon Feb 27, 2019

oliviertassinari Mar 4, 2019 •

edited

Loading

hburrows Mar 5, 2019

hburrows Mar 5, 2019

hburrows commented Mar 5, 2019

hburrows commented Mar 7, 2019

rahuliet commented Jun 18, 2019

[SpeedDial] Change actions background color #14640

[SpeedDial] Change actions background color #14640

Conversation

hburrows commented Feb 23, 2019 • edited by oliviertassinari Loading

mui-pr-bot commented Feb 23, 2019 • edited Loading

oliviertassinari commented Feb 23, 2019

mbrookes commented Feb 23, 2019 • edited Loading

hburrows commented Feb 24, 2019

mbrookes commented Feb 24, 2019

hburrows commented Feb 24, 2019

mbrookes commented Feb 24, 2019

hburrows commented Feb 27, 2019

mbrookes commented Feb 27, 2019

mbrookes Feb 27, 2019

Choose a reason for hiding this comment

hburrows Feb 27, 2019

Choose a reason for hiding this comment

hburrows Feb 27, 2019

Choose a reason for hiding this comment

eps1lon Feb 27, 2019

Choose a reason for hiding this comment

oliviertassinari Mar 4, 2019 • edited Loading

Choose a reason for hiding this comment

hburrows Mar 5, 2019

Choose a reason for hiding this comment

hburrows Mar 5, 2019

Choose a reason for hiding this comment

hburrows commented Mar 5, 2019

hburrows commented Mar 7, 2019

rahuliet commented Jun 18, 2019

hburrows commented Feb 23, 2019 •

edited by oliviertassinari

Loading

mui-pr-bot commented Feb 23, 2019 •

edited

Loading

mbrookes commented Feb 23, 2019 •

edited

Loading

oliviertassinari Mar 4, 2019 •

edited

Loading