This will afford us a couple of niceties

• Automatic version updates
• Automated changelog generation

To Do

• Configure semantic-release to use lerna in the project root on merge to master
• Configure changelog publishing on merge to master
• Create custom prepare/release plugin
  • Run lerna-publishduring prepare step
  • Push to npm during release step for each package

Forcing consumers of the DatePicker to pass a moment object in is a bit heavy-handed and offers unnecessary insight into the implementation of the DatePicker component. I think it would be beneficial to allow the DatePicker component to take in a Date or string and manage the conversions internally.

Thoughts?

The following warning is generated when using the Textbox component:
"styled.input is changing an uncontrolled input of type text to be controlled. Input elements should not switch from uncontrolled to controlled (or vice versa). Decide between using a controlled or uncontrolled input element for the lifetime of the component. More info: https://fb.me/react-controlled-components"

Probably just an easy initialization fix.

The current configuration is giving some false positives. For example, you can remove the propTypes objects from Button and npm run lint will still pass.

There are some accessibility issues that need to be addressed with the DropdownButton component:

1. Focus should be trapped within buttons until it is collapsed
2. Screen readers should be able to reliably state when the dropdown is expanded vs collapsed

The snapshots seem to be failing now because the styledcomponent names are more random. Because the snapshot includes them and they are being generated each time we can't get a passing build.

#72 introduces additional complexity to what call to action objects are accepted. CTA support should be ripped out of Notification into its own component in Patterns

Replace the solid underline in the popover sample to a dashed underline.

Currently we manually

• Set the version
• Update documentation
• Publish to npm

At least two of these items we can automate, docs updates and npm publish. The most heavy handed approach to documentation update would be to add another condition to the husky precommit hook. Travis can be configured to deploy to npm on master builds, as documented here. This should reduce the amount of work required in every single PR, at which point we can simply ask for a version bump upon an approved review.

The mantra so far has been that we need to supply the theme to a ThemeProvider only once and the changes will trickle down, but this becomes a little difficult to maintain when children contain their own ThemeProvider with a default prop value of defaultTheme. Currently simply passing the theme as a prop will resolve all these situations, but I wonder if we need to spend a little more time thinking about how we want to approach nested ThemeProviders in general.

Per documentation styled-component does provide a withTheme HoC that could resolve these issues in a potentially more elegant manner. To use #122 as an example:

import styled, { ThemeProvider, withTheme } from 'styled-components';
import ToggleButton from './path/to/ToggleButton';

const ThemedToggleButton = withTheme(ToggleButton);

My only issue with this approach is that this is effectively the same as the solution proposed in #122, only obfuscated by the withTheme HoC. On one hand, we define the theme for the theme provider once and don't pass it around, which we want, but on the other it's a bit heavy handed. I'm leaning a little towards using the HoC since it does make the jsx just slightly cleaner.

Another vein of thought to explore could be using withTheme in place of ThemeProvider in components we know for sure will be wrapped with ThemeProvider in some parent, but I have no idea what guarantees we have in avoiding the current problem.

Affected Locations

• #122
• #127

Currently, when a Popover contains a form element, the Popover will appear but never properly close. With the recent addition of the close button (#57) to non-form popovers, we need to extend the functioning close button to form popovers as well.

Some complications stemming from FocusTrap:

• Currently only surrounds the popover's body content, which leaves the close button out of reachable focus. It's the same when trying to click outside of the popover.
• FocusTrap cannot currently wrap Popover either, as this causes numerous breaks in both styling and popover function.

We need to update FocusTrap and/or PopoverTrigger to allow for the close button to be focusable/clickable.

Some nice upgrades in the next version. Should be a pretty simple update even if we don't use the additional features immediately.

https://medium.com/styled-components/announcing-v2-f01ef3766ac2

Ship a UMD build so we can use Unpkg to host over a CDN.

The only way to close the popover currently is by hitting ESC or through a script in the provided form (of which we should also have a sample). Add an "X" to the popover's upper-right corner to allow a more obvious method of closing it.

I don't think this really adds much value. I think we should just remove this all together. Thoughts?

If a consuming project is using the Textbox component and uses snapshot testing in Jest, the snapshot will consistently be updated causing failed snapshots over and over.

The cause of this is the genId() function call inside of the Textbox component. It should be safe to get rid of this call and use the name prop for the id of the input in order to maintain the for/id relationship which is important for accessibility.

Discovered with #211

Theoretically a ref can be used in place of ReactDOM.findDOMNode() for referencing a DOM node. Unfortunately with styled-components the innerRef prop passed to Textbox gets translated immediately to a ref prop.

Investigate utilizing this existing accessible component rather than creating a new one from scratch. Or at least utilize some of the ideas that make it accessible.

• add a 15px bottom margin on the leading text
• when an icon is included, line children up with the leading text following the icon and not underneath the icon

As a major focus for the UIs built for Willis Towers Watson is accessibility it would be smart to introduce a tool such as react-a11y to provide warnings/errors during development to promote accessible component creation.

While propType validation is disabled for production builds by default the propTypes object still remains. Even with tree shaking and minification these rules will still remain. babel-plugin-transform-react-remove-prop-types gives us some flexibility on how to approach this. Out of the box it will remove all instances of propTypes, but we only want this done for our downstream applications.

.babelrc

{
  "presets": [ "wtw-im" ],
  "plugins": [ ["transform-react-remove-prop-types", {
    "mode": "unsafe-wrap"
  }] ]
}

This config will transform this

Icon.propTypes = { 
  // icon propTypes
};

into

process.env.NODE_ENV !== 'production' ? Icon.propTypes = {
  // icon propTypes
} : void 0;

This maintains the dev time usefulness of propTypes while allowing them to be trimmed for production builds in downstream projects.

Thoughts?

Compiling assets when not using the datepicker (only importing a simple button):

import React from "react";
import ReactDOM from "react-dom";
import './styles/styles.css';

import Button from 'es-components';
import { ThemeProvider } from 'styled-components';
import { viaTheme } from 'es-components';

const App = () =>
  <div className="test">
    <h1>Colleague Portal</h1>
    <Button styleType="primary" isOutline style={buttonDemoStyle}>Primary</Button>
  </div>;

ReactDOM.render(
  <ThemeProvider theme={viaTheme}>
    <App />
  </ThemeProvider>,
  document.getElementById("app")
);

ERROR in ./node_modules/react-datepicker/es/index.js
Module not found: Error: Can’t resolve ‘moment’ in ‘/Users/ianlan/Projects/agent-management-suite/colleague-portal/app/node_modules/react-datepicker/es’
@ ./node_modules/react-datepicker/es/index.js 5:0-28 272:9-15 276:9-15 294:10-16 294:46-52 301:9-15 305:9-15 318:47-53 499:39-45 513:9-15 569:13-19 604:11-17 608:11-17 619:11-17 623:11-17
@ ./node_modules/es-components/lib/components/patterns/datepicker/DatePicker.js
@ ./node_modules/es-components/lib/index.js
@ ./src/index.js
@ multi (webpack)-dev-server/client?http://localhost:8080 webpack/hot/dev-server ./src/index.js

If we aren't using the datepicker in our project, this should not be a required dependency.

Using React 16.2

#147 introduces a fix for downstream packages to use the lib directory by default. Previously attempting to consume 10.7.1 will result in a handful of issues.

1. jest will attempt to compile components from this package. "transformIgnorePatterns" will allow es6 module packages such as the es6 build, lodash-es, etc., to be consumed without error.
2. Using "transformIgnorePatterns" results in fs errors in TeamCity per jestjs/jest#4444

At the moment of #147 we get around these issues by forcing downstream test projects to compile with the lib directory for test projects and the es6 directory for bundles. This is fine, but I wonder if there are ways to get jest to consume the es6 directory instead, so that we may eventually deprecate and remove the lib directory entirely.

See the announcement here.

styled-components changelog

Todo

• replace injectGlobal with createGlobalStyle
• replace .extend() with styled
• replace withTheme hoc

The above may not be final

The published package should only contain the built assets. We could probably trim a few other items from the release as well.

Just glancing at https://unpkg.com/[email protected]/

• src directory
• lib/styleguide directory - only needed to build the docs
• config directory - only needed to build the docs
• rebuild-docs.sh

The InputWrapper on the Textbox component is adding an 25px right margin, which prevents lining up fields with right-aligned buttons.

We have a work around, using the es-textbox__wrapper, css class, but I'd rather not have to reference the internal implementation to get what should be default behavior.

.es-textbox__wrapper
{
  margin-right: 0px !important;
}

It would be nice to have a link to the GitHub project from the published site (to help avoid some confusion).

This will allow us to migrate our .babelrc to .babelrc.js or babel.config.js. Moving to the new config formats will allow us to clean up our environment-based configs.

To Do

• Remove es-components-via-theme from depdendencies
• OOTB configuration differences between babel-preset-env and @babel/preset-env
• clean up default parameters

When trying to navigate the DatePicker component with keyboard, selecting a year or month causes the tab focus to move outside the DatePicker. Focus may need to be forced to the first element in the form after the selections are made.

Need more info from UX team.

update the buttons to display: block when the viewport is within mobile sizes.

Check with UX team for colors.

Show strings rather than integers for keys in at least one example.

According to the github insights dependency graph one of the packages has a vulnerability. At this moment there isn't anything to worry about, as the marked dependency is far down the tree and used only by our dev dependencies. In short, it does not impact any of our published packages.

In working with the components I have found it odd that the Via/WTW themes, live in this repo.

Does it make more sense for es-components to define the shape and required properties of a theme and have a default but then allow downstream users to define their own theme. It feels odd that a theme specific to any company lives in this repo. It also causes problems when needing to do any updates and small tweaks (color changes, screen size change, etc.), requiring a change here, an update in version, and a publish.

The styleguide works fine with the old PropTypes import from React, but when importing it from prop-types now, all props show up as "custom" in the styleguide build.

When working with the jsx-ally in eslint the following error appears

C:\Dev\Source\es-components\src\components\patterns\datepicker\DatePicker.specs.js
18:7 error onBlur must be used instead of onchange, unless absolutely necessary and it causes no negative consequences for keyboard only or screen reader users jsx-a11y/use-onblur-not-onchange

The challenge is I know onBlur and onChange do not do the same thing, and as such I do not see them being interchangeable. Do you need to change the lint rule globally, or do we make the change to our code?

With more dependencies beginning to use React 16 features as their defaults, it might make sense to start thinking about how this project can also move in that direction.

What are your thoughts on moving forward with intentions of removing any version of React <= 16 as a peer dependency in the next major version?

The close button is not selectable, not to mention it looks horrible.

When importing a library using styled-components into a project also using styled-components, conflicts occur. Due to this issue we need to move our styled-components dependency to peer & dev so the downstream app is only bundling one copy.

The drawer panel header should be decorated with [aria-controls="panel-id"], where panel-id is the id of the panel body div.

The three failing tests are all because of a recent update to the react-animate-height library. However, these fail only because the element is getting mounted in jsDOM and the DOM element ref does not have the style object on it. This should not affect any deployments. Not entirely sure yet if we should lock down to 0.9.9 like I already have with the node v8 test run or to continue floating it. Thoughts? @Darrken @jrios

When selecting year or month, you can then successfully click the title portion displaying the selected year or month and go back up a level. If you select a day, however, clicking on the title (e.g. February 2017) does nothing. If you click the left chevron arrows, it will then go back to the Month selection.

The current precommit hook can silently fail to regenerate the docs when the node_modules directory has been cleaned prior to commit. While the hook does reduce the previous overhead these silent errors can cause malformed documentation that is only caught when merged into master. We should replace this with some sort of bot to correctly generate and publish new documentation on merge to master.

For example, you can add an onKeyPress prop to a TextBox and it basically works as in the following example:

class TextboxExample extends React.Component {
  constructor() {
    this.state = { value: "" };
    this.handleOnTextChange = this.handleOnTextChange.bind(this);
    this.show = function(event) { if(event.key === 'Enter') console.log(event.key);};
  }

  handleOnTextChange(event) {
    console.log(event.target.value);
    this.setState({ value: event.target.value });
  }

  render() {
    return (
      <Textbox
        labelText="Controlled Example"
        value={state.value}
        onChange={this.handleOnTextChange}
        onKeyPress={this.show}
      />
    )
  }
}
<TextboxExample />

However, onKeyPress is not documented as one of the props at https://wtw-im.github.io/es-components/#textbox.

This led at least one person to believe TextBox didn't support onKeyPress.

Clearly it 'worked' because onKeyPress was passed to the underlying Input component via the ...additionalTextProps

We can integrate axe-core into our jest testing, as shown in their example. I don't know how well it will play with our current tests, since just about every component is mounted to jsdom.

Because there is no explicit usage an a or button within the SideNav.Item component, the navigation cannot be navigated or triggered using a keyboard. This is an accessibility concern for keyboard-only users.

As mentioned in #61 we probably need a little bit more thought in how we handle popovers in the mobile view. I keep thinking that we should probably default to using bottom placement always for mobile, that way the user is able to see the top of the content body. Your thoughts, @Darrken?

Besides babel 7 we have 5 outdated dependencies per david-dm, two of them we have pinned instead of floated.

Todo

• react-popper has been overhauled to use render props
• react-text-mask used PureComponent, which breaks our downstream projects
• text-mask-addons

react-text-mask and text-mask-addons are not floated dependencies. Fortunately we are only a couple versions behind and none of them are major. Can we float them to pull the latest v5 and v3, respectively?
react-animate-height and uncontrollable are far out of date. How much do we need to fix to get this updated?
react-popper released 1.0.2. It doesn't look like there are any "breaking" changes per the v1 release notes