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.

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

feat(color): add color component #592

Merged
merged 94 commits into from
Jul 25, 2020
Merged

Conversation

jcfranco
Copy link
Member

@jcfranco jcfranco commented May 18, 2020

Issue #64

This adds the following components:

  • calcite-color
  • calcite-color-hex-input
  • calcite-color-swatch

API

interface CalciteColor {
  // intl props omitted
  scale: "s" | "m" | "l";
  storageId: string;
  theme: "dark" | "light";
  value: ColorValue; 

  calciteColorChange: EventEmitter<void>;

  setFocus(): Promise<void>;
}

// CSS color | 
// { r: number, g: number: b: number} | 
// { h: number, s: number: v: number} |  
// { h: number, s: number: l: number};
type ColorValue = string | object; 

interface CalciteColorHexInput {
  hexLabel: string; // text used for aria-label
  scale: "s" | "m" | "l";
  theme: "dark" | "light";
  value: string;  // hex value

  onCalciteColorHexInputChange();

  setFocus(): Promise<void>;
}

interface CalciteColorSwatch {
  active: boolean;
  color: string;  // CSS color values
  scale: "s" | "m" | "l";
  theme: "dark" | "light";
}

Pending

  • Confirm naming convention of label props (intlProp?).
  • Update canvas rendering code for HD displays (looks a bit blurry in those cases)
  • Update tests
  • a11y audit
  • Add stories

Notes

  • The format of value will be preserved as the user makes updates (e.g., if user specifies value as rgb(), it will remain rgb() after making updates).
  • This adds https://www.npmjs.com/package/color as a dependency for working for color values.
  • This version does not handle alpha (will be handled in separate issue)
  • This also adds a new helper for testing component focus.
  • The change event has no detail object for the following reasons:
    • input value format is preserved (assumes input value is the desired output one)
    • color can change frequently and adding all of the possible color formats to the event seems unnecessary
    • secret :shipit: color prop is available for advanced color use cases
  • Hex values are normalized to be lowercase
  • Has groundwork for handling/preserving alpha values
  • Simplifies tsconfig.json.

Questions

  • What should the default color be? It's currently set to #007AC2.
  • Similar to calcite-date, should this be renamed to calcite-color?

@jcfranco jcfranco added the new component Issues tied to a new component. label May 18, 2020
@jcfranco jcfranco added this to the 🔨 v1-beta.27 milestone May 18, 2020
@jcfranco jcfranco self-assigned this May 18, 2020
@jcfranco jcfranco changed the title [DRAFT] Issue #64: Add calcite-color-picker [DRAFT] feat(color-picker): add color-picker component May 22, 2020
@macandcheese macandcheese removed this from the 🔨 v1-beta.27 milestone May 26, 2020
@jcfranco jcfranco added this to the 🔨 v1-beta.28 milestone Jun 1, 2020
@jcfranco
Copy link
Member Author

calcite-color is now dogfooding components! 🐶 ⬅ such a good boy!

The only outstanding item now is what to do with the small scale layout (see previous comment ☝️).

@paulcpederson
Copy link
Member

For now, in small scale we could just drop the r g b text, maybe?

Copy link
Member

@driskull driskull left a comment

Choose a reason for hiding this comment

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

Commentz

src/components/calcite-color/calcite-color.tsx Outdated Show resolved Hide resolved
src/components/calcite-color/calcite-color.tsx Outdated Show resolved Hide resolved
@bstifle
Copy link

bstifle commented Jul 24, 2020

ok @jcfranco here we go again...

sorry for the back and forth and universal sadness throughout this component.

But... here's the latest. Provided some spacing specs too. (all the typography is 14px)

any questions please let me know

Color Picker Sizes updated

Color Picker Spacing updated

@bstifle
Copy link

bstifle commented Jul 24, 2020

@jcfranco @macandcheese @julio8a

This is getting really confusing since we already have calcite-colors as a library.

Let's change the name back to calcite-color-picker

@macandcheese
Copy link
Contributor

calcite-color makes more sense to me... it's more consistent with calcite-date, and lends itself more to granular component use for things like calcite-color-swatch outside of the color picking context.

@TheBlueDog
Copy link

If you are using the scale system the graph height on M and L is slightly off.

Copy link

@bstifle bstifle left a comment

Choose a reason for hiding this comment

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

ship ittttt

@jcfranco jcfranco merged commit a146d52 into master Jul 25, 2020
@jcfranco jcfranco deleted the jcfranco/64-add-color-picker branch July 25, 2020 01:04
@jcfranco jcfranco mentioned this pull request Jul 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new component Issues tied to a new component.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants