Skip to content
This repository has been archived by the owner on Mar 14, 2021. It is now read-only.

Add aliases to colors palettes #13

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add aliases to colors palettes #13

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d0d5951
Add aliases to colors palettes
frycz Apr 6, 2020
eb310f8
Display aliases in colors component
frycz Apr 6, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/components/ColorSwatch.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { readableColor } from "polished";
import { Swatch, SwatchToken, SwatchValue } from "../";
import { tokenPropType, valuePropType } from "../propTypes";

export const ColorSwatch = ({ value, token }) => {
export const ColorSwatch = ({ value, token, aliases }) => {
const color = readableColor(value, "black", "white");
return (
<Swatch token={token} value={value}>
Expand All @@ -18,6 +18,11 @@ export const ColorSwatch = ({ value, token }) => {
}}
>
<SwatchToken color={color}>{token}</SwatchToken>
{aliases
? (Array.isArray(aliases) ? aliases : [aliases]).map(alias => (
<SwatchToken color={color}>{alias}</SwatchToken>
))
: null}
<SwatchValue color={color}>{value}</SwatchValue>
</div>
</Swatch>
Expand Down
2 changes: 1 addition & 1 deletion src/components/ColorSwatch.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
Renders a color swatch with a readable text.

```jsx harmony
<ColorSwatch token={"accent"} value={"#C0100F"} />
<ColorSwatch token={"accent"} aliases={"error"} value={"#C0100F"} />
Copy link
Member

Choose a reason for hiding this comment

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

Let's create a separate example for aliases. Also the name suggests it should be an array but it's a string. I also don't see any propTypes definitions for that prop.

```

When clicked, the value of the token is copied to clipboard.
19 changes: 14 additions & 5 deletions src/components/Colors.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { Grid, ThemeProvider } from "theme-ui";
import { omitBy, pickBy, isString } from "lodash";
import { Swatches, ColorSwatch, PaletteSwatch } from "../index";

export default function Colors({ theme }) {
export default function Colors({ theme, aliasesKey }) {
const gap = 2;
const colors = pickBy(theme.colors, isString);
const palettes = omitBy(theme.colors, isString);
Expand All @@ -19,7 +19,11 @@ export default function Colors({ theme }) {
gridTemplateColumns: "repeat(auto-fit, minmax(90px, 1fr))"
}}
>
<PaletteSwatch token={token} value={value} />
<PaletteSwatch
token={token}
value={value}
aliasesKey={aliasesKey}
/>
</Grid>
)}
</Swatches>
Expand All @@ -29,9 +33,14 @@ export default function Colors({ theme }) {
gridTemplateColumns: "repeat(auto-fit, minmax(90px, 1fr))"
}}
>
<Swatches theme={theme} items={colors}>
{(token, value) => (
<ColorSwatch token={token} value={value} key={token} />
<Swatches theme={theme} items={colors} aliasesKey={aliasesKey}>
{(token, value, aliases) => (
<ColorSwatch
token={token}
value={value}
aliases={aliases}
key={token}
/>
)}
</Swatches>
</Grid>
Expand Down
2 changes: 1 addition & 1 deletion src/components/Colors.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
```jsx harmony
import theme from "../theme";

<Colors theme={theme} />;
<Colors theme={theme} aliasesKey={"aliases"} />;
Copy link
Member

Choose a reason for hiding this comment

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

Same here. Leave the basic example and add an advanced one where you explain why it's like this. I don't like the aliasesKey prop name TBH since it's not self explanatory.. Again, prop types are missing.

```
11 changes: 8 additions & 3 deletions src/components/PaletteSwatch.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,17 @@ import PropTypes from "prop-types";
import { Swatches, ColorSwatch } from "../index";
import { tokenPropType, valuePropType } from "../propTypes";

export const PaletteSwatch = ({ token, value }) => (
<Swatches items={value}>
{(key, value) => (
export const PaletteSwatch = ({ token, value, aliasesKey }) => (
<Swatches items={value} aliasesKey={aliasesKey}>
{(key, value, aliases) => (
<ColorSwatch
value={value}
token={`${token}.${key}`}
aliases={
Array.isArray(aliases)
? aliases.map(alias => `${token}.${alias}`)
: aliases && `${token}.${aliases}`
}
key={`${token}.${key}`}
/>
)}
Expand Down
20 changes: 18 additions & 2 deletions src/components/PaletteSwatch.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,29 @@ const palette = [
"#C62828",
"#B71C1C"
];
<PaletteSwatch token={"red"} value={palette} />;
palette.aliases = [
"lightest",
"light",
"diabled",
"inactive",
"regular",
"hovered",
"errorLight",
["error", "failure"],
"errorDark"
];

<PaletteSwatch token={"red"} value={palette} aliasesKey="aliases" />;
Comment on lines +17 to +29
Copy link
Member

Choose a reason for hiding this comment

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

This example is overwhelming for me. Here is an example from the spec: https://styled-system.com/theme-specification#space which I'd suggest using.

```

as well as object notation:

```jsx harmony
import theme from "../theme";

<PaletteSwatch token={"slate"} value={theme.colors.slate} />;
<PaletteSwatch
token={"slate"}
value={theme.colors.slate}
aliasesKey="aliases"
/>;
```
20 changes: 17 additions & 3 deletions src/components/Swatches.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,25 @@
import React from "react";
import PropTypes from "prop-types";

const Swatches = ({ items = [], children }) => (
const Swatches = ({ items = [], aliasesKey, children }) => (
<>
{Array.isArray(items)
? items.map((value, index) => children(index, value))
: Object.keys(items).map(key => children(key, items[key]))}
? items.map((value, index) =>
children(
index,
value,
aliasesKey && items[aliasesKey] ? items[aliasesKey][index] : null
)
)
: Object.keys(items)
.filter(key => key !== aliasesKey)
.map(key =>
children(
key,
items[key],
aliasesKey && items[aliasesKey] ? items[aliasesKey][key] : null
)
)}
Comment on lines +7 to +22
Copy link
Member

Choose a reason for hiding this comment

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

TBH I'm not sure I want to accept this code into the repo. Maybe the best course of action would be to do this in your repo after all. You can compose things with Swatches and Swatch, add the resolution logic on top of what'e being done and extend provided primitives.

Thoughts?

Copy link
Author

Choose a reason for hiding this comment

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

Maybe we can just use this repo to create a custom library? The only component which can be reused without any change is Swatch (which gives copy-to-clipboard feature)

Copy link
Member

@okonet okonet Apr 7, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not sure yet. I'd like to support aliases but I'm not sure the code complexity is worth it. Is there a simpler way of getting aliases for tokens? Ideally, as the user, I should not care about providing alias keys to the library. Let me poke with it a bit and let you know.

</>
);

Expand Down
10 changes: 9 additions & 1 deletion src/theme.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,15 @@ const palette = {
base: "#2E3D49",
light: "#6D7780",
lighter: "#B4B9BD",
lightest: "#F7F7F8"
lightest: "#F7F7F8",
aliases: {
darker: "foused",
dark: "active",
base: "default",
light: "hovered",
lighter: "disabled",
lightest: "inactive"
}
Comment on lines +13 to +20
Copy link
Member

Choose a reason for hiding this comment

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

Why did that change?

}
};
export default {
Expand Down