[docs] Add color selector #12053

mbrookes · 2018-07-03T23:32:28Z

Customise the docs colors, and create palettes for use in createMuiTheme().

Closes #2009

oliviertassinari · 2018-07-04T07:59:28Z

docs/src/pages/style/color/color.md


-<a href="https://material.io/tools/color/#!/?view.left=0&view.right=0&primary.color=3F51B5&secondary.color=F44336">


Why removing this image? I find it complementary to the demo you added. It's providing context and guidance on how to use the tool.

Because this tool effectively replaces it. We're linking to the general Material Design color section rather than their color tool. We don't need a picture of theirs alongside ours.

I did wonder if I should add something to display all the generated dark and light colors along those lines.

Because this tool effectively replaces it

I don't think that the new tool answers the same problems. I don't see how it can be a replacement. Let me explain what I see.

tools/color provides:

A official theme color picker

A tool to help to pick accessible colors

Force us to keep in sync with their palette model

the new demo provides:

A way for users to see how the palette impacts all our components: solve [docs] Mapping color to components #2009

Vuetify has been trying to replace the official color picker, I don't think that it execute. I think that it's too much effort for the outcome. They would have been better off showing how to use the official pickers, a better output for less effort: https://vuetifyjs.com/en/theme-generator. At least, it's how I see it from an external point of ~~vue~~ view. @johnleider would you mind sharing your experience with the theme generator? :)

I prefer to keep developers on the docs as much as possible. Our jobs as framework developers is to bring all the good important stuff to the front. As this is a visual demo, I find it has a lot of value. This is why we are rebuilding ours :)

So what?

Wow, that accessibility tab has almost zero discoverability.

How so?

As you know, I started with the ability to copy the URL from the MD color tool and paste it in a text field, but it was an extremely cumbersome way to try different color schemes with our components, hence the addition of the color pickers, to the point where I dropped the URL text field. I can add it back if you see value in it - but I'd love some analytics to see if it gets used!

Users can always populate the theme from the official color picker if they choose, and we can keep an explanation of that.

@johnleider Thanks for dropping by and sharing your view, you're doing awesome work over there!

but I'd love some analytics to see if it gets used!

Good point, we can add one Google Analytics event.

Users can always populate the theme from the official color picker if they choose, and we can keep an explanation of that.

As long as we have some documentation around how to do such, I'm happy!

oliviertassinari · 2018-07-04T08:22:08Z

packages/material-ui/src/styles/createPalette.js

@@ -125,7 +125,7 @@ export default function createPalette(palette: Object) {
    return contrastText;
  }

-  function augmentColor(color, mainShade, lightShade, darkShade) {
+  function augmentColor(color, mainShade = 500, lightShade = 300, darkShade = 700) {


oliviertassinari · 2018-07-04T08:24:23Z

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

@@ -33,7 +33,7 @@ export const style = theme => {
    root: {
      position: 'relative',
      width: '100%',
-      margin: '10px 0',
+      margin: '10px',


Why do we have a margin by default :o.

The way the thumb is currently constructed, without it, the active thumb would overlap surrounding components. This was a quick fix for the horizontal overlap.

I guess it's tangentially related to #11889.

Should we be using the padding instead? Using the margin is deceptive. One can't override it directly.

Yeah, it's better that way.

oliviertassinari · 2018-07-04T08:24:24Z

docs/src/pages/style/color/color.md

      main: '#3f50b5',
-      dark: '#002884',
-      contrastText: '#fff',


Do we document this feature somewhere else?

It's covered in detail the "Themes" section, but we could add a note / cross reference here.

Alright, then the only advantage is to illustrate how to use the output of material.io/tools/color/.

oliviertassinari · 2018-07-04T08:25:18Z

docs/src/pages/style/color/ColorTool.js

+
+export default compose(
+  withStyles(styles, {
+    name: 'ColorChooser',


We most likely don't need to give the style sheet a name.

oliviertassinari · 2018-07-04T08:32:47Z

docs/src/pages/style/color/ColorDemo.js

+
+const ColorDemo = props => {
+  const { classes, primary, theme, secondary } = props;
+  theme.palette.augmentColor(primary);


Hum, this is not immutable.

No, and it confused the heck out of me at first. It probably shouldn't have been added to the public API in that form (or written that way in the first place - my bad), but changing it now would be a breaking change.

Yes, I screw up. We will sort that out in the next breaking change release. I'm keeping track of all the breaking changes we might introduce, I have 9 so far.

oliviertassinari · 2018-07-04T11:10:37Z

docs/src/pages/style/color/color.md

      main: '#3f50b5',
-      dark: '#002884',
-      contrastText: '#fff',


Alright, then the only advantage is to illustrate how to use the output of material.io/tools/color/.

oliviertassinari · 2018-07-04T11:11:44Z

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

@@ -33,7 +33,7 @@ export const style = theme => {
    root: {
      position: 'relative',
      width: '100%',
-      margin: '10px 0',
+      margin: '10px',


Should we be using the padding instead? Using the margin is deceptive. One can't override it directly.

oliviertassinari · 2018-07-04T11:12:32Z

packages/material-ui/src/styles/createPalette.js

@@ -1,6 +1,6 @@
 import warning from 'warning';
 import deepmerge from 'deepmerge'; // < 1kb payload overhead when lodash/merge is > 3kb.
-import indigo from '../colors/indigo';
+import blue from '../colors/blue';


oliviertassinari · 2018-07-04T11:13:39Z

docs/src/pages/style/color/ColorTool.js

+
+const shades = [900, 800, 700, 600, 500, 400, 300, 200, 100, 50, 'A700', 'A400', 'A200', 'A100'];
+
+export const styles = theme => ({


No need to export the styles

Oops no - same copypasta as the stylesheet naming :)

oliviertassinari · 2018-07-04T11:14:11Z

docs/src/pages/style/color/ColorTool.js

+import actionTypes from 'docs/src/modules/redux/actionTypes';
+import ColorDemo from './ColorDemo';
+
+const hues = [


Can you get this list out of Object.keys(colors)?

oliviertassinari · 2018-07-04T11:17:27Z

docs/src/pages/style/color/ColorTool.js

+    secondaryShade: 11,
+  };
+
+  hashPrefix = string => (string.charAt(0) === '#' ? string : `#${string}`);


Why using a class method over a function on the root scope?

oliviertassinari · 2018-07-04T11:17:39Z

docs/src/pages/style/color/ColorTool.js

+
+  hashPrefix = string => (string.charAt(0) === '#' ? string : `#${string}`);
+
+  isRgb = string => /#?([0-9a-f]{6})/i.test(string);


Why using a class method over a function on the root scope?

class methods can access the instance. I try to promote the function that doesn't need it to the top level to increase the isolation between the different sections of the code.

oliviertassinari · 2018-07-04T11:18:29Z

docs/src/pages/style/color/ColorTool.js

+    });
+  };
+
+  handleChangeDocsColors = values => {


handleChangeDocsColors = event => {?

oliviertassinari · 2018-07-04T11:20:35Z

docs/src/pages/style/color/ColorDemo.js

+
+const ColorDemo = props => {
+  const { classes, primary, theme, secondary } = props;
+  theme.palette.augmentColor(primary);


Yes, I screw up. We will sort that out in the next breaking change release. I'm keeping track of all the breaking changes we might introduce, I have 9 so far.

oliviertassinari · 2018-07-04T11:21:52Z

docs/src/pages/style/color/ColorTool.js

+export default compose(
+  withStyles(styles, {
+    name: 'ColorChooser',
+    withTheme: true,


oliviertassinari

The layout of the Color tool demo is off for smaller screens. The new iteration on this pull request is really awesome 😍 !

oliviertassinari · 2018-07-10T21:28:21Z

docs/src/modules/components/AppFrame.js

+            <Tooltip id="appbar-color" title="Edit docs colors" enterDelay={300}>
+              <IconButton
+                color="inherit"
+                href="/style/color/#color-tool"


Better use the next.js link. I can do it if you want.

I tried, but it doesn't seem to respect the hash (or I"m doing it wrong). It links to the page, but not to the section. You have to click twice - once to go to the page, once to go to the section.:

<IconButton color="inherit" component={buttonProps => ( <Link variant="button" href="/style/color/#color-tool" {...buttonProps} /> )} aria-labelledby="appbar-color" >

oliviertassinari · 2018-07-10T21:29:45Z

docs/src/pages/style/color/ColorDemo.js

+  },
+});
+
+const ColorDemo = props => {


I try to use function everywhere in the codebase for react components.

Add color swatches Checkmark for selected colors Fix imports Add shade slider basically working Fix indigo glitch

Does this make the code to dense / hard to read?

* [docs] Add color chooser Add color swatches Checkmark for selected colors Fix imports Add shade slider basically working Fix indigo glitch * Add ColorDemo component * Remove google color tool support * Fix TextField input * Rename ColorChooser to ColorTool * Improve the layout * Address the rest of Olivier's feedback * Add toolbar icon * Add docs-style-color to regression test blacklist * Optimize sample palette * Use MarkdownElement for code sample * Add dark / main / light colorBar * Display hex values in colorBar * fix code layout * Fix size limits * DRY the colorPicker Does this make the code to dense / hard to read? * Hacky fix for layout at small screen size * looking at the PR

artivilla · 2018-07-16T21:08:55Z

Can someone link me to this page on the documentation? Thank you for this :)

oliviertassinari · 2018-07-16T21:15:48Z

@artivilla https://material-ui.com/style/color/#color-tool

mbrookes · 2018-07-17T11:53:07Z

@artivilla You also have a new icon in the docs site Toolbar:

artivilla · 2018-07-17T19:50:10Z

Just to confirm, this only allows us to modify primary/secondary base colors? Not the entire theme file.

oliviertassinari · 2018-07-17T20:05:46Z

@artivilla Exactly

oliviertassinari reviewed Jul 4, 2018

View reviewed changes

oliviertassinari reviewed Jul 10, 2018

View reviewed changes

mbrookes added 15 commits July 11, 2018 03:11

[docs] Add color chooser

2820c3a

Add color swatches Checkmark for selected colors Fix imports Add shade slider basically working Fix indigo glitch

Add ColorDemo component

36e7797

Remove google color tool support

47affdb

Fix TextField input

0e6a2c9

Rename ColorChooser to ColorTool

92f1409

Improve the layout

d3657f6

Address the rest of Olivier's feedback

7b69c83

Add toolbar icon

c4cee0f

Add docs-style-color to regression test blacklist

d1f522c

Optimize sample palette

820dd19

Use MarkdownElement for code sample

618a55b

Add dark / main / light colorBar

c6df3bd

Display hex values in colorBar

91eca03

fix code layout

faf03f9

Fix size limits

b7cd54a

oliviertassinari self-assigned this Jul 11, 2018

mbrookes and others added 3 commits July 11, 2018 12:45

DRY the colorPicker

7de1477

Does this make the code to dense / hard to read?

Hacky fix for layout at small screen size

9e4f439

looking at the PR

34b93f6

oliviertassinari merged commit 355317f into mui:master Jul 11, 2018

mbrookes deleted the docs-custom-colors branch July 17, 2018 11:53

snyk-bot mentioned this pull request Sep 30, 2019

[Snyk] Upgrade @material-ui/core from 1.0.0 to 1.5.1 Matthelonianxl/nifty-wallet#7

Open

This was referenced Mar 26, 2020

[Snyk] Upgrade @material-ui/core from 1.0.0 to 1.5.1 AndreiShybin/metamask-extension#9

Merged

[Snyk] Upgrade @material-ui/core from 1.0.0 to 1.5.1 baophucct/metamask-extension#5

Open


		<a href="https://material.io/tools/color/#!/?view.left=0&view.right=0&primary.color=3F51B5&secondary.color=F44336">


		const shades = [900, 800, 700, 600, 500, 400, 300, 200, 100, 50, 'A700', 'A400', 'A200', 'A100'];

		export const styles = theme => ({


		hashPrefix = string => (string.charAt(0) === '#' ? string : `#${string}`);

		isRgb = string => /#?([0-9a-f]{6})/i.test(string);

[docs] Add color selector #12053

[docs] Add color selector #12053

Conversation

mbrookes commented Jul 3, 2018 • edited Loading

oliviertassinari Jul 4, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oliviertassinari Jul 4, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mbrookes Jul 4, 2018 • edited Loading

Choose a reason for hiding this comment

oliviertassinari Jul 4, 2018 • edited Loading

Choose a reason for hiding this comment

oliviertassinari Jul 4, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oliviertassinari left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mbrookes Jul 10, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

artivilla commented Jul 16, 2018

oliviertassinari commented Jul 16, 2018

mbrookes commented Jul 17, 2018

artivilla commented Jul 17, 2018

oliviertassinari commented Jul 17, 2018

mbrookes commented Jul 3, 2018 •

edited

Loading

oliviertassinari Jul 4, 2018 •

edited

Loading

oliviertassinari Jul 4, 2018 •

edited

Loading

mbrookes Jul 4, 2018 •

edited

Loading

oliviertassinari Jul 4, 2018 •

edited

Loading

oliviertassinari Jul 4, 2018 •

edited

Loading

mbrookes Jul 10, 2018 •

edited

Loading