Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Color system] Icon #2781

Merged
merged 7 commits into from
Mar 3, 2020
Merged

[Color system] Icon #2781

merged 7 commits into from
Mar 3, 2020

Conversation

danrosenthal
Copy link
Contributor

@danrosenthal danrosenthal commented Feb 26, 2020 •
edited
Loading

What this does

Addresses: https://github.com/Shopify/polaris-ux/issues/348

Updates the Icon component to consume the new design language. It adds the following as new color options:

  • base : default in new design language
  • subdued
  • critical
  • warning
  • highlight
  • success
  • primary

And aliases all old design language color options to logical equivalents in the new design language.

Questions for reviewers

  • should we have disabled, hovered, and pressed colors added to the icon color API?
    • decided to remove the interaction-state-related color options
  • did I make the right choices in aliasing the existing values?
    • decided to alias sky and white to subdued, ink and black to base, and purple to highlight
    • if we have second thoughts after this is merged, we can always change without it being considered breaking
  • should twotone icons use --p-icon-on-interactive (always white) or --p-surface (variable) as the accent color?
    • decided on using --p-surface
  • is it the right decision to omit an interactive color for icon?
    • decided this is the right decision

What this looks like

old design language new design language - light new design language - dark
Screen Shot 2020-02-27 at 12 14 39 PM Screen Shot 2020-02-27 at 12 14 45 PM Screen Shot 2020-02-27 at 12 14 50 PM

Playground

import React from 'react';

import {CircleCancelMinor, HomeMajorTwotone} from '@shopify/polaris-icons';
import {Page, Icon, Heading, Stack} from '../src';

export function Playground() {
  return (
    <Page title="Playground">
      <Stack>
        <Heading>New monotone</Heading>
        <Icon source={CircleCancelMinor} />
        <Icon source={CircleCancelMinor} color="base" />
        <Icon source={CircleCancelMinor} color="subdued" />
        <Icon source={CircleCancelMinor} color="critical" />
        <Icon source={CircleCancelMinor} color="warning" />
        <Icon source={CircleCancelMinor} color="highlight" />
        <Icon source={CircleCancelMinor} color="success" />
        <Icon source={CircleCancelMinor} color="primary" />
      </Stack>
      <hr />
      <Stack>
        <Heading>Old monotone</Heading>
        <Icon source={CircleCancelMinor} color="white" />
        <Icon source={CircleCancelMinor} color="black" />
        <Icon source={CircleCancelMinor} color="skyLighter" />
        <Icon source={CircleCancelMinor} color="skyLight" />
        <Icon source={CircleCancelMinor} color="sky" />
        <Icon source={CircleCancelMinor} color="skyDark" />
        <Icon source={CircleCancelMinor} color="inkLightest" />
        <Icon source={CircleCancelMinor} color="inkLighter" />
        <Icon source={CircleCancelMinor} color="inkLight" />
        <Icon source={CircleCancelMinor} color="ink" />
        <Icon source={CircleCancelMinor} color="blueLighter" />
        <Icon source={CircleCancelMinor} color="blueLight" />
        <Icon source={CircleCancelMinor} color="blue" />
        <Icon source={CircleCancelMinor} color="blueDark" />
        <Icon source={CircleCancelMinor} color="blueDarker" />
        <Icon source={CircleCancelMinor} color="indigoLighter" />
        <Icon source={CircleCancelMinor} color="indigoLight" />
        <Icon source={CircleCancelMinor} color="indigo" />
        <Icon source={CircleCancelMinor} color="indigoDark" />
        <Icon source={CircleCancelMinor} color="indigoDarker" />
        <Icon source={CircleCancelMinor} color="tealLighter" />
        <Icon source={CircleCancelMinor} color="tealLight" />
        <Icon source={CircleCancelMinor} color="teal" />
        <Icon source={CircleCancelMinor} color="tealDark" />
        <Icon source={CircleCancelMinor} color="tealDarker" />
        <Icon source={CircleCancelMinor} color="greenLighter" />
        <Icon source={CircleCancelMinor} color="green" />
        <Icon source={CircleCancelMinor} color="greenDark" />
        <Icon source={CircleCancelMinor} color="yellowLighter" />
        <Icon source={CircleCancelMinor} color="yellow" />
        <Icon source={CircleCancelMinor} color="yellowDark" />
        <Icon source={CircleCancelMinor} color="orange" />
        <Icon source={CircleCancelMinor} color="redLighter" />
        <Icon source={CircleCancelMinor} color="red" />
        <Icon source={CircleCancelMinor} color="redDark" />
        <Icon source={CircleCancelMinor} color="purple" />
      </Stack>
      <hr />
      <Stack>
        <Heading>New twotone</Heading>
        <Icon source={HomeMajorTwotone} />
        <Icon source={HomeMajorTwotone} color="base" />
        <Icon source={HomeMajorTwotone} color="subdued" />
        <Icon source={HomeMajorTwotone} color="critical" />
        <Icon source={HomeMajorTwotone} color="warning" />
        <Icon source={HomeMajorTwotone} color="highlight" />
        <Icon source={HomeMajorTwotone} color="success" />
        <Icon source={HomeMajorTwotone} color="primary" />
      </Stack>
      <hr />
      <Stack>
        <Heading>Old twotone</Heading>
        <Icon source={HomeMajorTwotone} color="white" />
        <Icon source={HomeMajorTwotone} color="black" />
        <Icon source={HomeMajorTwotone} color="skyLighter" />
        <Icon source={HomeMajorTwotone} color="skyLight" />
        <Icon source={HomeMajorTwotone} color="sky" />
        <Icon source={HomeMajorTwotone} color="skyDark" />
        <Icon source={HomeMajorTwotone} color="inkLightest" />
        <Icon source={HomeMajorTwotone} color="inkLighter" />
        <Icon source={HomeMajorTwotone} color="inkLight" />
        <Icon source={HomeMajorTwotone} color="ink" />
        <Icon source={HomeMajorTwotone} color="blueLighter" />
        <Icon source={HomeMajorTwotone} color="blueLight" />
        <Icon source={HomeMajorTwotone} color="blue" />
        <Icon source={HomeMajorTwotone} color="blueDark" />
        <Icon source={HomeMajorTwotone} color="blueDarker" />
        <Icon source={HomeMajorTwotone} color="indigoLighter" />
        <Icon source={HomeMajorTwotone} color="indigoLight" />
        <Icon source={HomeMajorTwotone} color="indigo" />
        <Icon source={HomeMajorTwotone} color="indigoDark" />
        <Icon source={HomeMajorTwotone} color="indigoDarker" />
        <Icon source={HomeMajorTwotone} color="tealLighter" />
        <Icon source={HomeMajorTwotone} color="tealLight" />
        <Icon source={HomeMajorTwotone} color="teal" />
        <Icon source={HomeMajorTwotone} color="tealDark" />
        <Icon source={HomeMajorTwotone} color="tealDarker" />
        <Icon source={HomeMajorTwotone} color="greenLighter" />
        <Icon source={HomeMajorTwotone} color="green" />
        <Icon source={HomeMajorTwotone} color="greenDark" />
        <Icon source={HomeMajorTwotone} color="yellowLighter" />
        <Icon source={HomeMajorTwotone} color="yellow" />
        <Icon source={HomeMajorTwotone} color="yellowDark" />
        <Icon source={HomeMajorTwotone} color="orange" />
        <Icon source={HomeMajorTwotone} color="redLighter" />
        <Icon source={HomeMajorTwotone} color="red" />
        <Icon source={HomeMajorTwotone} color="redDark" />
        <Icon source={HomeMajorTwotone} color="purple" />
      </Stack>
    </Page>
  );
}

@github-actions
Copy link
Contributor

github-actions bot commented Feb 26, 2020 •
edited
Loading

🔴 This pull request modifies 6 files and might impact 136 other files. Because this is a larger than average splash zone for a change, remember to tophat areas that could be affected.

Details:
All files potentially affected (total: 136)
🎨 src/components/Button/Button.scss (total: 53)

Files potentially affected (total: 53)

🎨 src/components/Frame/components/Toast/Toast.scss (total: 4)

Files potentially affected (total: 4)

🎨 src/components/Icon/Icon.scss (total: 84)

Files potentially affected (total: 84)

🧩 src/components/Icon/Icon.tsx (total: 83)

Files potentially affected (total: 83)

🧩 src/components/Icon/tests/Icon.test.tsx (total: 0)

Files potentially affected (total: 0)

🧩 src/types.ts (total: 136)

Files potentially affected (total: 136)

@danrosenthal danrosenthal added the 🤖Skip Changelog Causes CI to ignore changelog update check. label Feb 27, 2020
danrosenthal
danrosenthal commented Feb 27, 2020
View reviewed changes
src/components/Button/Button.scss Show resolved Hide resolved
src/components/Button/Button.scss Show resolved Hide resolved
src/components/Frame/components/Toast/Toast.scss Show resolved Hide resolved
src/components/Frame/components/Toast/Toast.scss Show resolved Hide resolved
src/components/Icon/Icon.scss Outdated Show resolved Hide resolved
src/components/Icon/Icon.tsx Outdated Show resolved Hide resolved
src/components/Icon/Icon.tsx Show resolved Hide resolved
src/components/Icon/Icon.tsx Show resolved Hide resolved
src/components/Icon/Icon.tsx Show resolved Hide resolved
src/types.ts Outdated Show resolved Hide resolved
@danrosenthal danrosenthal changed the title [WIP][Color system] icon [Color system] Icon Feb 27, 2020
tmlayton
tmlayton approved these changes Feb 27, 2020
View reviewed changes
Copy link
Contributor

@tmlayton tmlayton left a comment

Choose a reason for hiding this comment

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

I’m good with this pending any feedback about the color mappings.

danrosenthal
danrosenthal commented Mar 3, 2020
View reviewed changes
src/components/Icon/Icon.scss
}

.colorBlueDark {
@include color-icon('blue', 'dark');

&::before {
background-color: color('blue', 'light');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Forgot to write a note about this earlier. This value is not a supported backdrop, and must have been missed in previous changes

@danrosenthal
Copy link
Contributor Author

danrosenthal commented Mar 3, 2020 •
edited
Loading

I paired this back a bit. Removing the interaction-state-related color options as I'm less confident about their usefulness.

I'm also feeling confident about the aliasing, and in the fact that we can always change it if we have second thoughts.

Going to merge this on green once I've made recommended notes for design review.

@danrosenthal danrosenthal merged commit 3fc257f into master Mar 3, 2020
@danrosenthal danrosenthal deleted the color-system-icon branch March 3, 2020 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tmlayton tmlayton tmlayton approved these changes

@sarahill sarahill Awaiting requested review from sarahill

@mirualves mirualves Awaiting requested review from mirualves

@martenbjork martenbjork Awaiting requested review from martenbjork

@alex-page alex-page Awaiting requested review from alex-page

@kyledurand kyledurand Awaiting requested review from kyledurand

@sadiesaurus sadiesaurus Awaiting requested review from sadiesaurus

@VanjaMitrovic VanjaMitrovic Awaiting requested review from VanjaMitrovic

Assignees
No one assigned
Labels
🤖Skip Changelog Causes CI to ignore changelog update check.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@danrosenthal @tmlayton