🚀 Feature Proposal

Create a new convenience component that wraps Tooltip with our Popper helper component.

This change could be additive by simply overlaying the Popper props on top of Tooltip. If the Popper props aren't used, then the Popper.js positioning engine wouldn't be used and the consumer could implement their own positioning engine if desired.

In addition, the functionality for triggering and hiding the tooltip on hover/focus/blur should be implemented.

Motivation

To provide the consumer with a more robust and easier to consume tooltip experience.

Example

<Tooltip placement={'right'} anchorElement={this.buttonRef.current}>
  Close
</Tooltip>

🚀 Feature Proposal

Update border radius to 8px (ideally using #58) as per the design spec.

Pulled from #51, and this will be a breaking visual change.

Motivation

As part of the evolution of Canvas, border radii are increasing! Many of our components have increased in radius including Card (which ripples to Popup, Modal, Dialog, etc.)

🐛 Bug Report

Children in Layout will mistakenly get cloned with spacing as a prop set by Layout. Only Column should have spacing passed in by Layout.

The logic here doesn't check for the type of React child properly.

To Reproduce

See Codesandbox

Expected Behavior

Layout should ignore non Column type components and not clone it with the spacing prop.

Actual Results

Layout incorrectly identifies children as Columns and clones that child with an extra spacing prop if Layout defines a spacing prop.

Browser (if applicable)

If this was a visual bug, what browser did you spot it on?

Link to repl or repo (highly encouraged)

https://codesandbox.io/s/clever-haibt-vnkim

Error Output

Paste the error output here

or

Include a screenshot of a visual bug

🚀 Feature Proposal

It would be really nice if the Layout component supported stacking. Right now the only way you can achieve this is by using two different Layout components. Also, it would be great if having more than 12 columns inside a layout caused it to wrap or gave you a warning.

🚀 Feature Proposal

It would be nice to be able to recognize contributors on the PRs that they made in our release notes (Changelog.md) through Github tagging.

Motivation

👍 Contributors are awesome

Example

fix: Update lodash version to resolve vulnerability (#41) (@vibdev)

🚀 Feature Proposal

Right now SidePanel opens and closes on top of content. It would be nice to have a layout component that could re-flow the content around the side panel when it opens. I think it makes sense to use the existing Layout component for this.

Layout.LeftPanel and Layout.RightPanel would render a SidePanel, passing any event listeners necessary for Layout to manage the state. Maybe the existing Layout component becomes Layout.Grid?

Note: for performance reasons, we should also look at doing these transitions with translation rather than animating the width of SidePanel

Example

<Layout /* Add a prop to animate width vs. post translate */>
  <Layout.LeftPanel open={true} sidePanelBackgroundColor={SidePanelBackgroundColor.Gray}>
    <ul>
      <li>Link</li>
    </ul>
  </Layout.LeftPanel>
  <Layout.Grid>
    <Layout.Column>
      <Card heading="Column 1" />
    </Layout.Column>
    <Layout.Column>
      <Card heading="Column 2" />
    </Layout.Column>
  </Layout.Grid>
  <Layout.RightPanel open={true} sidePanelBackgroundColor={SidePanelBackgroundColor.Gray}>
    <ul>
      <li>Link</li>
    </ul>
  </Layout.RightPanel>
</Layout>

🚀 Feature Proposal

Form labels should show an asterix for those fields that are required.

Color should use cinammon500 (canvas.inputColors.error)

Example

<FormField label="Label" inputId="my-checkbox-field" required={true}>
  <Checkbox id="1" label="Checkbox option" />
</FormField>

🛠 🚀

(this might be lower priority given that we don't make a ton of new components anymore)

Proposal

Upgrade create-module.sh for better automation and extensibility. We also have a golden opportunity to ensure consistency and standards through this CLI. We can do this by demonstrating these standards through generated boilerplate code and reducing manual work by contributors.

Open Questions

• Which command line parser should we use? Commander is the most popular, yargs is pirate-themed. Then there's minimist (the guts of optimist... who now recommends using yargs).

General Architecture

• Convert create-module.sh to node. Use a modern command line parser like commander, yargs, or minimist

package.json

• Add emotion and jest-emotion as dep and devDep respectively

canvas-kit-react

• Add component to the universal module canvas-kit-react as an import
• Add component to package.json

index.ts

• Change all instances of MyComponent to the component named in the prompt as the default (author can change this later in unusual cases). e.g. page-header should be PageHeader component

MyComponent.tsx

• Rename the file name to the PascalCase version (i.e. PageHeader)
• Rename the component name to the PascalCase version
• Add component interface (i.e. PageHeaderProps)
• ... augment React.Component<P, S> with it.
• Instead of a <div> as the boilerplate, use a styled component instead passing in the prop generic as well. i.e. const Container = styled('div')<PageHeaderProps>({...}, (props) => ({...}))

README.md

• Title is "Page-header" today. It should be something like "Page Header"
• We should generate an outline for the readme (Description, usage, props section) to promote consistency
• Add a line that says "TODO: fill out readme" in the body of the description
• Add a linter rule that checks for the above line in the README and fail it if it exists

stories.tsx

• Import is MyComponent.tsx today, should match the changes noted above so it's the actual file name of the component. i.e. PageHeader.tsx
• Change storiesOf to the correct component name (i.e. Canvas Kit/Page Header not Canvas Kit/Page-header)
• Default the section-label to the component name
• Insert the correct component tag (i.e. instead of <MyComponent> use <PageHeader>)

Tests

While we're adding basic unit tests that pass, the test suite will still fail because coverage won't be at the level we need.

• Create directory for tests (spec or tests)
• Add basic unit tests ('component should render') that pass and provide a skeleton with all the dependencies needed
• Add basic snapshot test that provides a skeleton with all the dependencies needed
• Generate snapshots jest -u
• Update packge.json script's test command
• Add ability to run tests on a single module
• Add helpful hints and quick tips in the comments section of stubbed out tests
• Add component folder to test coverage in jest.config.js

🚀 Feature Proposal

A while ago we added support for easily customizing the margin and padding of component containers (see here). We haven't done much with this, but we should be taking advantage of it in the container of most, if not all, of our components.

We may need to discuss restricting certain props in some way. For example, consumers should be able to adjust the margin around an avatar, but adjusting the padding doesn't make sense.

Pulled from #51, but this is not breaking.

Motivation

Many consumers need to adjust the spacing within/around the container element of our components. Currently they either have to use a wrapping div, or a style attribute - neither are ideal due to the extra markup they create. This API was created to add an easy way to adjust this spacing using normal props.

We should probably also standardize on the variable name used for these styled components as well (I'm thinking Container)

Example

import {space} from '@workday/canvas-kit-react-core';

// ...

const Container = styled('div')(space)<MyComponentProps>(
  // my styles
)

// ... 

export default class MyComponent extends React.Component<MyComponentProps> {
  // ...
  render() {
    const {myProp, ...elemProps} = this.props;

    return (
      <Container myProp={myProp} {...elemProps}>
        ...
      </Container>
    )
  }
}

// Consumer
<MyComponent p={spacing.xl} pb={64} m={40} mx={10}>
  ...
</MyComponent>

🐛 Bug Report

We never really benchmarked the focusRing memoization to make sure that it works the way we intended it to be. In an effort to simplify the codebase, I want to keep track of this issue and figure out what we're really gaining from memoization and if there's another way we could do this.

🐛 Bug Report

TypeError when child of layout returns null

To Reproduce

<Layout gutter={0}>
  {
    conditional &&
    <Layout.Column>
      <div>
        <Component />
      </div>
    </Layout.Column>
  }
  <Layout.Column>
    <Component>
      <h2>
        {conditional ?  ${variable} : ""}
      </h2>
      {
        condtional && <Component />
      }
    </Component>
  </Layout.Column>
  {
    conditional && (
      <Layout.Column>
        {moreCode}
      </Layout.Column>
    )
  }
</Layout>

This code is okay:

<Layout gutter={0}>
  <>
    {
      conditional &&
      <Layout.Column>
        <div>
          <Component />
        </div>
      </Layout.Column>
    }
    <Layout.Column>
      <Component>
        <h2>
          {conditional ?  ${variable} : ""}
        </h2>
        {
          conditional && <Component />
        }
      </Component>
    </Layout.Column>
    {
      conditional && (
        <Layout.Column>
          {moreCode}
        </Layout.Column>
      )
    }
  </>
</Layout>

Expected Behavior

<Layout.Column> should handle null children

Actual Results

TypeError is thrown

Browser (if applicable)

Chrome

Error Output

TypeError: Cannot read property 'type' of null
Layout._this.renderChild
node_modules/@workday/canvas-kit-react-layout/dist/es6/lib/Layout.js:58
  55 | function Layout() {
  56 |     var _this = _super !== null && _super.apply(this, arguments) || this;
  57 |     _this.renderChild = function (child) {
> 58 |         if (typeof child.type === typeof Column) {
     | ^  59 |             var childProps = child.props;
  60 |             if (childProps.spacing || childProps.spacing === 0) {
  61 |                 return child;

🚀 Feature Proposal

To get a better understanding of the Storybook we're looking at we should be displaying a version in an obvious location.

Motivation

This will make looking at a storybook and using a component stay in sync mentally if features or bug fixes got added.

Example

For example, it would be nice to know what package version was represented when you looked at an Input or Button component.

Summary

We should consolidate our usage of font-smoothing. We have varied use across the kits, with a mix of antialiased, subpixel-antialiased and auto. These cause our text to render differently depending on the component.

I don't know if font smoothing is something we've ever officially agreed upon with design. We should do a review of the options across different browsers with them and decide on one to use in all contexts.

🚀 Feature Proposal

Lightweight video player. This player would support html5 video and would have having minimal default buttons (maybe play/pause, volume, captions, and full screen). I think there could be some discussion about if we could allow users to extend the list of buttons to add their own functionality.

Posible Features

• showControls / hideChrome
• setInitialPlayerState
  • isPaused
  • isMuted
  • volume
  • playbackRate
  • currentTime
  • quality
  • width
  • height
  • backgroundColor
• getting and setting playerState
• controlling playback
  • player.play()
  • player.pause()
  • player.mute()
  • player.unmute()
  • player.toggleFullscreen()
• adding listeners for player events
  • player.listen('timeupdate' ...)
  • player.listen('pause' ...)
  • player.listen('seeked' ...)

Motivation

There has been some interest in a fast lightweight experience that just does video playback.

Example

Minimum would be something like

<VideoPlayer
  source={sourceURL}
>

But you could imagine something a bit more complex like

<VideoPlayer
  source={sourceURL},
  width={width}
  height={height}
  background={posterImageUrl}
  subtitles={subtitlesObject}
>

🐛 Bug Report

Versions of lodash before 4.17.12 are vulnerable to Prototype Pollution. The function defaultsDeep allows a malicious user to modify the prototype of Object via {constructor: {prototype: {...}}} causing the addition or modification of an existing property that will exist on all objects.

Remediation

commitizen and jest-axe also have this vulnerability and need to be updated. Their repos have not received a patch yet though. They are dev deps and are not published with modules so they are less of a concern in the short term.

https://www.npmjs.com/advisories/1065

🐛 Bug Report

Switch does not change state well on firefox

To Reproduce

Steps to reproduce the behavior:
Toggle the switch in your app on firefox

Expected Behavior

Nice animation (blue outline around switch component)

Actual Results

Small square box pops under the switch

Browser (if applicable)

Firefox 68.0.1 (64-bit) on mac os 10.14.5

🚀 Feature Proposal

Based on design's input, going forward we are only going to support one type of loading animation. We need to remove LoadingSpinner from @workday/canvas-kit-react-loading-animations

Pulled from #51, and this is a breaking change

Motivation

The LoadingSpinner and LoadingDots serve an identical purpose right now and we need to consolidate. They came about from various design teams on various products. We originally implemented both to support all use cases, but are encouraging consumers to converge on one option. Going forward we will only support the LoadingDots.

🚀 Feature Proposal

It would be cool if PageHeader could be customized to achieve results similar better support what is available in our product:

It would be really convenient if it had component that wrapped the left/right side, and common parts like title/sub-title like:

function MyPageHeader() {
  return (
    <PageHeader title="Hello">
      <PageHeader.TitleContainer>
        <PageHeader.Title>View Flows</PageHeader.Title>
        <PageHeader.SubTitle>HackerRank <Button /></PageHeader.SubTitle>
      </PageHeader.TitleContainer>
      <PageHeader.Navigation>
        <IconButton icon={exportIcon} />
        <IconButton icon={fullscreenIcon} />
      </PageHeader.Navigation>
    </PageHeader>
  )
}

Motivation

To avoid reimplementing (almost) the same component with custom children/make the PageHeader a bit more customizable for use cases when a title and controls are not enough

🚀 Feature Proposal

Add a link to the component documentation on design.workday.com for each component in every component readme.

Motivation

We have a component status readme that links to usage documentation on the design site, but it's more of an overall view. When looking at a single component, it's a pain to jump back and forth between technical and usage documentation, and this would make it slightly easier.

Example

https://design.workday.com/components/buttons/buttons would be linked to in https://github.com/Workday/canvas-kit/blob/master/modules/button/react/README.md and https://github.com/Workday/canvas-kit/tree/master/modules/button/css

🐛 Bug Report

Modals are placed on a dark background, but have a gray border from the nested Card
component. Modal renders a Popup, which then renders the Card. This gray border makes the edge look blurry. This could potentially be an issue when placing cards on any dark or colored background (i.e. the design.workday.com).

Expected Behavior

While we want the gray border on Cards and Popups, there should be no border on Modals.

🚀 Feature Proposal

A few components are not exporting all their enums as static variables. Banner, Select, and SidePanel are the culprits.

Pulled from #51, but is not breaking.

Example

See https://github.com/Workday/canvas-kit/blob/master/API_PATTERN_GUIDELINES.md#static-class-variables

🚀 Feature Proposal

As part of the evolution of Canvas, border radii are increasing! Many of our components have increased in radius, and we now have a system for choosing between 2px, 4px, and 8px.

These three values (along with '0' and '100%') should be exported from both the React and CSS core modules.

We could probably use tshirt sizes (s, m, and l) for the three sizes:

For React: consider how to capitalize these re. #51 (comment). This matches how we've been doing it elsewhere, but they're not PascalCase.

As part of this, an audit should be conducted to retroactively update any places where these constants should be used.

Example

import {canvas, borderRadius} from '@workday/canvas-kit-react';

const MyRoundedBox = styled('div')({
  borderRadius: borderRadius.l
})

const MyCircle = styled('div')({
  borderRadius: canvas.borderRadius.circle
})

SCSS:

border-radius: $wdc-border-radius-m;

🐛 Bug Report

1. Header spreads html attributes props to multiple elements. This causes issues with style, id, auto-ids, etc
2. Search magnifying glass is not clickable to search
3. Search clear button (x) is not accessible and can not be tabbed to.

🚀 Feature Proposal

Add a global InputProviderDecorator to Storybook's config.js using addDecorator.

Motivation

Right now we're manually adding InputProviderDecorator to every story file and it's easy to forget. This would enforce it on every story.

Example

In .storybook/config.js

import {InputProviderDecorator} from '@workday/canvas-kit-react-common';

addDecorator(InputProviderDecorator);

🐛 Bug Report

Code coverage does not include any files with a .ts extension. As a result some utility functions are missing tests.

To Reproduce

Steps to reproduce the behavior:

• run yarn test
• inspect the code coverage results

Expected Behavior

Files with .ts extension are included in the results. (Note: files with a .d.ts extension should still be excluded).

Actual Results

Only files with a .tsx extension are included.

🚀 Feature Proposal

Button needs several breaking changes to bring it in line with our API & Pattern Guidelines:

• Rename buttonType to type
• Rename buttonSize to size
• Rename all types to singular
• Rename all static variables to singular
• Re-evaluate prop spread behavior (let's discuss further)
• Make aria-label required for IconButton
• Consider scaling down Small SquareFilled Icon Button icon to match the other Small icon
• Fix the focus outline on square buttons to match others (it's the only one that doesn't have the white ring inside the blue) (see #53 (comment)).

Ensure you document all breaking changes that are made in your PR.

Pulled from #51 and will be a breaking change.

Motivation

We want our APIs to be as consistent as possible and Button is currently one of the most divergent components. Let's bring it back in line by making these changes so our APIs feel more natural from one component to the next.

🐛 Bug Report

The Avatar component needs some documentation updates and we need to move {...elemProps} to the end so we can support overriding the aria label of the button.

To Reproduce

Steps to reproduce the behavior:
Try to add altText and an aria-label to the avatar component. The altText prop will override the aria label, therefore missing the aria label.

Expected Behavior

The consumer should be able to pass an alt text for an image, and an aria label in the avatar component if it's a button.

Include a screenshot of a visual bug

🚀 Feature Proposal

Add a new optional href prop to Button components:

  /**
   * The URL to link to when the button is clicked. 
   * If specified, will use an `a` element as the root of the component
   */
  href?: string;

It would also probably be good to add some prop that gets used as the target attribute on the underlying a tag

Motivation

Many of our use cases of the Button component end up essentially being links, changing the current URL in the onClick handler:

<Button onClick={() => window.location.href='url'}>Button Text</Button>

// may also use `window.open('url', '_blank')` in some cases

This has downsides in that it doesn't benefit from any of the native browser functionality that actual links get when using the a tag, such as being able to explicitly open a link in a new tab or window

One workaround to get the benefits of an a tag, while also getting the style of the Button component is to wrap the Button in an a:

<a href='url' target='_blank'>
  <Button>Button Text</Button>
</a>

The downsides to this approach:

• The a tag may have styles that need to be suppressed (at the very least, disabling its outline)
• The a and underlying button elements are both focusable, causing keyboard nav to first focus the a and then the button next.

Example

<Button href='http://www.google.com'>Button Text</Button>

// DOM renders as: 
<a href='http://www.google.com'>
  /* ... children elements */
</a>

// while maintaining the style of the original `button` based implementation.

🐛 Bug Report

canvas-kit-react only exports canvas-kit-react-core's canvas namespace so we don't provide access to objects not contained in canvas. For example, canvas-kit-react-core is exporting spacingNumbers generally, but not under the canvas namespace.

To Reproduce

import {canvas} from '@workday/canvas-kit-react';

canvas.spacingNumbers; // undefined

or

import {spacingNumbers} from '@workday/canvas-kit-react'; // import error

Expected Behavior

spacingNumbers should be available from canvas-kit-react universal module.

We don't want to export all types/enums on canvas, so I think we should re-export everything from the core module in the universal one since that's what we do for all other modules.

🐛 Bug Report

FormField's LabelPoition and ErrorBehavior enums should be changed to FormFieldLabelPosition and FormFieldErrorBehavior to prevent naming clashes.

For the reasoning, see 3rd point on https://github.com/Workday/canvas-kit/blob/master/API_PATTERN_GUIDELINES.md#enums.

However, these should still use the same static class variables (i.e. FormField.LabelPosition)

This is pulled from #51 and is a breaking change

🐛 Bug Report

We are missing @workday/canvas-kit-react-menu in our _canvas-kit-react/package.json file 😞

🚀 Feature Proposal

Move from validate-commit-msg to commitlint

Motivation

validate-commit-msg has been deprecated in favor of commitlint: https://www.npmjs.com/package/validate-commit-msg. It's working fine as is though, so this is a low priority

🐛 Bug Report

I can't inspect an icon button (IconButton.tsx) using buttonRef because buttonRef prop is not passed.

To Reproduce

Steps to reproduce the behavior:
Use an IconButton including a buttonRef prop with a callback such as (ref) => { console.log(ref) }

Expected Behavior

Callback should be called

Actual Results

Callback not called

Browser (if applicable)

Any

Link to repl or repo (highly encouraged)

Please provide a reference to a repo or branch to help us reproduce this bug.

Error Output

none

🐛 Bug Report

Currently Icon accepts an elemProps prop and each wrapping component passes elemProps instead of spreading them. This should be updated so that it's more consistent with our other components (see https://github.com/Workday/canvas-kit/blob/master/API_PATTERN_GUIDELINES.md#prop-spread-behavior)

This is from #51, but is not a breaking change.

Example

AppletIcon.tsx

public render() {
  const {icon, color, size, ...elemProps} = this.props;

  return (
    <Icon
      src={icon}
      type={CanvasIconTypes.Applet}
      styles={appletIconStyles({color})}
      size={size}
      {...elemProps}
    />
  );
}

Icon.tsx

public render() {
  const {src, size, styles, type, ...elemProps} = this.props;

  // ...

  return <Svg src={src} type={type} styles={iconStyles} {...elemProps} />;
}

As part of the 3.0 Alpha, we are making some breaking changes to solidify our APIs, particularly in accordance with our API & Pattern Guidelines.

The following are proposed API changes. This should probably move to our RFC repo once a public one is created.

API Changes

Universal Changes

• (#95 / #150) Not Breaking: Rename props (ActionBar), inputProps (TextArea, TextInput) and otherProps to elemProps

• Maybe Breaking (#162) Change HTMLAttributes usage to HTMLProps @lychyi

• Add a beta/sandbox/unstable module @anicholls

Component Changes

Avatar

• (#194) Change themeColor to variant @stephanerangaya

Banner

• (#96 / #110) Not Breaking: Export enums as static variables
• (#195) Change theme to variant. @stephanerangaya

Button

• (#97 / #129) Rename buttonType to type
• (#97 / #129) Rename buttonSize to size
• (#97 / #129) Rename all types to singular
• (#97 / #129) Rename all static variables to singular
• (#97 / #129) Re-evaluate prop spread behavior
• (#97 / #129) Make aria-label required for IconButton
• (#97 / #129) Consider scaling down Small SquareFilled Icon Button icon to match the other Small icon
• (#97 / #129) Fix the focus outline on square buttons to match others (it's the only one that doesn't have the white ring inside the blue) (see #53 (comment))
• Promoting beta_Button to Button and deprecating Button @mannycarrera4

Card

• (#98) / (#204) Update border radius (ripples to Popup, etc) @stephanerangaya

ColorPicker

• (#99 / #150) Not Breaking: For ColorPicker, pass elemProps to ColorPreviewComponent

Core

• Move beta_type to sandbox @sahlhoff (blocked by @anicholls PR)
• Make spacing enums singular @mannycarrera4

Form Field

• (#100 / #146) Should LabelPosition, ErrorBehavior, etc. be prefixed with component name? We do this elsewhere
• (#99 / #150) Add elemProps

Header

• (#157) Split search out into a combobox component
• Header needs a larger refactor. Will come back to this.
• Move to unstable/sandbox module @sahlhoff (blocked by @anicholls sandbox PR and @vibdev combox PR)

Layout

• (#99 / #150 ) Not Breaking: Unpack props for both components

Loading-animation

• (#103 / #142) Deprecate loading spinner
• (#99 / #150) Not Breaking: Add elemProps

Menu

• Use Popup so you don't have to wrap once we have a11y fixes @NicholasBoll
• Move to unstable/sandbox @sahlhoff (blocked by @anicholls PR)

Modal

• (#99 / #150) Not Breaking: Add elemProps
• Remove transformOrigin prop. Modals should always animate from the bottom up @NicholasBoll

Page Header

• (#104 / #138) Change marketing prop

Popup

• (#110) Capitalize Popup.padding static variable
• (#59) A11y updates @NicholasBoll

SidePanel

• (#105 / #123) Change sidePanelBackgroundColor to backgroundColor
• (#105 / #123) Not Breaking: add backgroundColor as a static variable
• (#105 / #123) Not Breaking: Lots of broken styles (undefinedpx), etc.
• (#105 / #123) Not Breaking: Change open param in onBreakingChange to isAboveBreakpoint or something similar. We shouldn't suggest the behavior.

Skeleton

• (#99 / #150) Not Breaking: Add elemProps?

Switch

• (#197) Not Breaking: Add a11y messaging in Readme (the same as checkbox) @stephanerangaya

Table

• #172 TableRow
  • Singularize TableRowStates @sahlhoff
  • Update to use common error/alert enums and colors @sahlhoff
  • (#99 / #150) NOT BREAKING: Add elemProps

🐛 Bug Report

We have various names for elemProps and we should standardize on one (see https://github.com/Workday/canvas-kit/blob/master/API_PATTERN_GUIDELINES.md#prop-spread-behavior).

Pulled from #51, but not breaking.

Cases that need to be updated:

• ActionBar uses props
• TextArea and TextInput use inputProps
• Various components use otherProps

I think this is all, but good to do another pass.

🐛 Bug Report

We need to remove the event listener when `SidePanel unmounts. Right now it erroneously adds another event listener.

https://github.com/Workday/canvas-kit/blob/master/modules/side-panel/react/lib/SidePanel.tsx#L141

🚀 Feature Proposal

Add a CI check (maybe using GitHub Actions?) to check the PR title for conventional commit style.

Bonus points: Let's start reviewing PRs with the thought in mind that the title of the PR will be the merge commit message (which will ultimately be used in the changelog, so it should be very clear!)

Motivation

Since we are moving to squash merges, the default squash commit message will be the title of the PR. We would like these commits to follow our standard conventional commit style.

🐛 Bug Report

When trying to commit partially staged files, our linting adds the entire file during precommit without saying anything. I often accidentally commit lines I purposefully excluded because of this and end up with messy commits (or I have to re-write my local history).

To Reproduce

Steps to reproduce the behavior:

• Make some changes to a file
• Run git add -p
• Select some lines and not others
• Run git commit

Expected Behavior

My commit should only contain the changes I've staged.

Actual Results

Your commit will include the lines you excluded

Solution

We are currently on lint-staged v7.1. Support was added for partially staged files in v8.0: https://hackernoon.com/announcing-lint-staged-with-support-for-partially-staged-files-abc24a40d3ff

🚀 Feature Proposal

Rename marketing prop in PageHeader to capWidth (matches Layout)

This is from #51 and is a breaking change.

Motivation

We are dropping all notions of the "Marketing" type of components. Workday Canvas is for all contexts are we are trying to remove the differentiation between product and non-product components. Previously we had used the term marketing to denote non product patterns since they were usually used in marketing sites. We would like to move away from this and be more descriptive about what's actually being affected (in this case, "capping" the width with a max-width).

🐛 Bug Report

Popup does not currently pass accessibility requirements. We cannot use it for many cases unless we add the ability to lock/cycle focus within them, and focus on the first focusable item when opened, etc.

We need to support everything in https://bitsofco.de/accessible-modal-dialog/ except for # 4:

1. Markup the Dialog and Dialog Overlay Appropriately
2. On Dialog Open, Set Focus
3. On Dialog Close, Return Focus to the Last Focused Element
4. While Open, Prevent Tabbing to Outside the Dialog
5. While Open?
6. Allow the ESC Key to Close the Dialog

🐛 Bug Report

Some folks are getting the following warning in the console and also running the tests:

Using kebab-case for css properties in objects is not supported. Did you mean WebkitFontSmoothing?

Line 139 of StatusIndicator is the culprit: '-webkit-font-smoothing': 'antialiased',. It needs to be changed to `WebkitFontSmoothing.

It's still working as intended, but we should resolve the warning by making this change.

🚀 Feature Proposal

SidePanel needs a breaking change to bring it in line with our API & Pattern Guidelines:

• Change sidePanelBackgroundColor to backgroundColor
• Not Breaking: Add backgroundColor as a static variable (#110)
• Not Breaking: Change open param in onBreakpointChange to aboveBreakpoint. We shouldn't suggest the behavior - consumers can do what they want with the return value.
• Bonus: There are lots of undefined styles being applied based on conditionals in the styled components. Let's fix this 🙂

Ensure you document all breaking changes that are made in your PR.

Pulled from #51 and will be a breaking change.

Motivation

We want our APIs to be as consistent as possible. Let's bring SidePanel back in line by making these changes so our APIs feel more natural from one component to the next.

🐛 Bug Report

Perception-wise, the diagonal sheen of the Skeleton loader does not fire immediately when the component renders. Given the 5 second duration of the animation, it has a considerable delay before you see it. If the placeholder is meant to substitute content for sub-second load times, most users will never see the sheen.

To Reproduce

https://workday.github.io/canvas-kit/?path=/story/skeleton--complete

Expected Behavior

The sheen should animate as soon as the component renders/appears.

Actual Results

The sheen takes a bit before it animates. Long enough so that content may already replace what's behind it before you see it.

🐛 Bug Report

The fonts module should be opt-in so consumers can choose whether or not they consume things from our CDN (it's the only module that pulls directly from that URL). Currently it's being exported by default from @workday/canvas-kit-react and it should not be.

🐛 Bug Report

Several components can not pass props to their containers using our prop spread method. We will need to extend the interface as well.

Components/modules missing elemProps:

• FormField
• LoadingAnimation
• Modal
• Skeleton
• Table
• ColorPreview (inside @workday/canvas-kit-react-color-picker)

Additionally, Layout and Layout.Column aren't following this pattern because they're spreading all their props (i.e. {...this.props} vs. const {spacing, gutter, capWidth, children, ...elemProps} = this.props). These should be unpacked, with elemProps added to make it clear which props are additionally being added.

This is pulled from #51, but is not a breaking change.

To Reproduce

<ColorPreview value="#00FFCC" data-attribute="hello" />

Expected Behavior

The data attribute should appear on the resulting element.

Actual Results

The data attribute does not appear on the resulting element.

🚀 Feature Proposal

Consolidate breakpoints across the React and CSS kits and put them in the respective core modules as constants.

Motivation

We have some inconsistent breakpoints that we should align on. Also it would be nice to have constants for breakpoints for consumers to depend on. This would make projects implemented with the Canvas Kits more consistent across the board.

This could also potentially be combined with the makeMq helper for the React kit.

🚀 Feature Proposal

Extend our button set to include an inverse primary button.

Motivation

Allow more flexibility for marketing sites and design teams to use a primary call to action on darker blue backgrounds. This is especially needed on dark blue backgrounds where the current primary blue button would disappear in the background and the outline inverse button doesn't have enough prominence.

Example

Specs

🐛 Bug Report

We only accept a string in the toast, but this means no text can be bold, italic, etc. Ideally we would support this kind of styling.

https://github.com/Workday/canvas-kit/blob/master/modules/toast/react/lib/Toast.tsx#L14

Potential solutions:

• Allow ReactNode as a child. Unfortunately changing this to allow any node allows people to put block level elements inside of toast which we would prefer to avoid.
• Accept a string of markdown for the contents?

🐛 Bug Report

There are various warnings being logged in four test files. Currently our tests pass as long as there are no errors, but we should aim to have no warnings when running tests.

To Reproduce

Run yarn test

Error Output