Blocked by #29 and #38

In the long-run we may need to integrate this with our CI tools, but an immediate solution to ensuring code quality would be to add a pre-commit hook to run tests and the linter.

Cursor is still a pointer

I would also posit that we don't sufficiently distinguish between disabled components and active components (in general), the color change is barely noticeable and the cursor change is only noticed when mousing over. A more clear distinction would improve UX

Should act similarly to EuiCheckboxGroup since radios only show up in multiples.

Sometimes we need to style a link like a button, which KuiButton can't do. react-bootstrap takes the approach of switch what element is rendered depending on whether an href is passed, so we could take that approach perhaps. I don't know if that gets tricker with e.g. react-router.

I think Popover might be the only case where I smell code in this otherwise pristine library.

Popover examples look like this:

default class extends Component {
  state = {
    isPopoverOpen: false,
  }

  onButtonClick() {
    this.setState({
      isPopoverOpen: !this.state.isPopoverOpen,
    });
  }

  closePopover() {
    this.setState({
      isPopoverOpen: false,
    });
  }

  render() {
    const button = (
      <EuiButton
        iconType="arrowDown"
        iconSide="right"
        onClick={this.onButtonClick.bind(this)}
      >
        Show popover
      </EuiButton>
    );

    return (
      <EuiPopover
        ownFocus
        button={button}
        isOpen={this.state.isPopoverOpen}
        closePopover={this.closePopover.bind(this)}
      >
        <div style={{ width: '300px' }}>Popover content that&rsquo;s wider than the default width</div>
      </EuiPopover>
    );
  }
}

Some issues I see here:

• Hierarchy: the button is a tree-child of the popover, but information-wise the popover is the child, since you can't ever see it if you don't click the button
• Coupling: I should be able to connect popovers to all kinds of things, not just buttons specifically. What if I want more control, like a popover on hover or a popover for autocomplete that shows up in some way I want to program up?
• Even though we have coupled popover and the button that it's "appended to", we still have to manually manage state to open/close and determine whether the popover is open/closed

Having looked at the HTML, that answers the first problem: the popover creates a DOM node that wraps the button, when the popover is open a panel is shown next to it, and CSS gives it the effect that makes it pop over the button. So far so good.

I realize this is a subtle problem with a complicated UI component, but to me we should consider either of:

• Have the <EuiPopover> handle open/close state on its own
• Untangle the components by doing something like:

<EuiPopover popup={ MyPopup }>
  {
    ({ isOpen, toggle }) =>
      <button onClick={ toggle }>
        { isOpen ? 'Close' : 'Open' }
      </button>
  }
</EuiPopover>

function MyPopup({ close }) {
  return <div>
    <h1>This is what pops up</h1>
    <button onClick={ close }>Close</button>
  </div>
}

What I like about such approach is that we'd force the consumer to decouple the popover contents from the trigger, we simplify the state management by handling most of it, we don't implicitly name the trigger heavily suggesting it should be a button, and the popup can live separate from the, e.g, menu bar that's probably not all that related to the contents.

Similarly to the trigger mechanism, we can pass a close prop to MyPopup, maybe expecting ({ close }) => <MyPopup close={ close } /> or a functional component directly, which takes it as a prop without any middleware functions

Thoughts? @cjcenizal

Maybe it should default to size="m" since that appears to be the most commonly occurring size?

@bevacqua @pugnascotia Funny didn't notice this till now, but it doesn't look like the CSS compiles on a change to the SCSS files. If you can help me out there I can take on a bunch of those issues in the issue list. Sorry, I'm pretty oblivious to Webpack so I don't know how to trigger it.

Maybe EuiFormHelpText wasn't intended to be widely used outside of EUI, but we did adopt it extensively in our Cloud experiment, and it looks a bit off when added right below a progress like so:

  <span>
    <EuiProgress value={ used } max={ total } />
    <EuiFormHelpText>
      <FormattedMessage
        id='allocated-feature-stats-table.available-capacity'
        defaultMessage='{available} of {total}'
        values={ {
          available: prettySize(available),
          total: prettySize(total)
        } } />
    </EuiFormHelpText>
  </span>

Otherwise even simple things like showing a dropdown or a tooltip in a table row are going to be quite problematic:

@snide already has a design

Need to make modals responsive. Will likely need to fix the height of the body to trap the scrolling behavior to the modal itself, then remove the masking / container for the modal so it's just a single "page" action.

Implementation:

<a href={href} className={classes} {...rest}>
  <EuiIcon
    className="euiHeaderLogo__icon"
    type="logoKibana"
    size="xLarge"
    title="Go to Kibana home page"
  />
</a>

It should allow the user to set the type (and, perhaps, default to logoElastic or at least to nothingness), and it should not have the Go to Kibana title, which can be provided by the consumer.

We have SVG logos for most of our products, but not Elasticsearch.

In this snippet:

The issue is that when we want to render full-width cells such as Node distribution in the following table:

We need to do something like .ourTable .kuiTableCellContent__text { width: 100% }. Is this reasonable, or should we have a prop to turn off the text mode, whereby we don't add the extra span highlighted here:

I'd rather not have consumers need to dig into the internal classes as much as possible (in the context of React components at least), since that's the value in React for the most part, abstracting the classes away

In conclusion, would it be possible to do something like:

{
  this.props.allowOverflow === true
    ? children
    : <span className='kuiTableCellContent__text'>{ children }</span>
}

We need a CI job on Travis or similar to build eui.now.sh on every change to master. I think this is the right approach at this time.

Alternatively, long-term, I would say we need a deploy task that takes care of bumping the package version, publishing to npm, and deploying to now.

As noted in #54, we use "text" and "default" to refer to similar palette colors, which could be confusing for people.

Badge uses "default":

ButtonEmpty uses "text":

My suggestion is to just drop "text" and use "default" instead, since a prop of color="text" doesn't mean much to me, especially in the case of <KuiTextColor color="text">.

Just for convenience.

Add proper docs for textOnly prop on table cells. Likely need to explain their usage better in the written documentation.

#26 (comment)

@cjcenizal @bevacqua Was thinking

<EuiCode format="block">{code}</EuiCode>

with inline being the default format?

Then maybe adding a highlight prop that will do highlight.js rendering? Might also need a size prop for size of the text used?

I have the styling in my head, but if you have any specific ideas for how you want this componentized lemme know.

Leads to confusing behavior where the styling isn't properly applied. Probably related to #8 and root cause may yield to styles in screenshot:

So that when placed side-by-side with small buttons, for example, they are aligned

Per @zumwalt's suggestion, we could use @supports to provide a fallback for IE11.

Changing state should not be possible if the switch is disabled via <KuiSwitch disabled={ someCondition } />.

Let's simplify the repo history by only allowing commits when they've been squashed (https://github.com/blog/2141-squash-your-commits).

If we're going to do this, it needs to be now. It's only going to get worse, and since this isn't just meant for Kibana anymore, I think <Kui* is going to be confusing going forward.

So that import paths in code blocks can be readily copy pasted without any postprocessing

Given we now have static builds, it's better to use GitHub pages as a solution to #21 instead.

When disabled, <KuiSwitch> should be dimmed or otherwise represent inability to be clicked or interacted with. /cc @snide

Related #5

This may be just me misusing the component library, but when we have:

<EuiFieldText
  value={ currentValue }
  onChange={ e => this.updateValue(e.target.value) } />

<EuiText>
  <FormattedMessage
    id='region-settings-index.specify-a-cname-description'
    defaultMessage='Specify a CNAME that replaces the default hostname. This CNAME is used to modify your cluster and Kibana endpoints.' />
</EuiText>

It renders them kind of super close:

I'm 100% fine if this is not intended to be optimal in the context of EUI.

Most other components, e.g <EuiButton>, have a size property with values like 'small', 'large', but <EuiSpacer> relies on 's', 'm', etc shorthands. These should be consistent because principle of least surprise

Thus, ID propType should be required.

Currently there's extra padding from the empty contents:

However this is inconsistent with the toast component which supports no text:

cc @snide

Something about <FocusTrap>, then the entire app goes blank.

We currently do

.euiIcon { fill: black }
.euiIcon__basic { fill: black }  // which I'm not sure why we even apply since the main class already does this

I suggest that we consider { fill: currentColor } as that's way easier to work around without mushing with CSS

Thoughts @snide?

From the header bar example:

<span className='euiHeader__notification'>
  3
</span>

Expected interface:

<EuiHeaderNotification>3</EuiHeaderNotification>

We use PostCSS as part of the build process, but its config lives under docs/ at the moment. We should move it somewhere more appropriate.

@bevacqua I think this one is yours:

We need to add NPM scripts for running these tasks. In Kibana, these scripts are defined as:

    "uiFramework:createComponent": "yo ./ui_framework/generator-kui/app/component.js",
    "uiFramework:documentComponent": "yo ./ui_framework/generator-kui/app/documentation.js"

I think createComponent and createDocumentation might be better names. Suggestions welcome.

Otherwise clicking submits the form, which is rarely what you want

For reference, here's the config we're using in Kibana:

{
  "rootDir": "../../",
  "roots": [
    "<rootDir>/src/core_plugins",
    "<rootDir>/ui_framework/"
  ],
  "collectCoverageFrom": [
    "ui_framework/src/components/**/*.js",
    "!ui_framework/src/components/index.js",
    "!ui_framework/src/components/**/*/index.js",
    "ui_framework/src/services/**/*.js",
    "!ui_framework/src/services/index.js",
    "!ui_framework/src/services/**/*/index.js"
  ],
  "moduleNameMapper": {
    "^ui_framework/components": "<rootDir>/ui_framework/components",
    "^ui_framework/services": "<rootDir>/ui_framework/services",
    "^ui_framework/src/test": "<rootDir>/ui_framework/src/test",
    "^ui/(.*)": "<rootDir>/src/ui/public/$1",
    "\\.(jpg|jpeg|png|gif|eot|otf|webp|svg|ttf|woff|woff2|mp4|webm|wav|mp3|m4a|aac|oga)$": "<rootDir>/src/jest/mocks/file_mock.js",
    "\\.(css|less|scss)$": "<rootDir>/src/jest/mocks/style_mock.js"
  },
  "setupFiles": [
    "<rootDir>/src/jest/setup/request_animation_frame_polyfill.js"
  ],
  "coverageDirectory": "<rootDir>/target/jest-coverage",
  "coverageReporters": [
    "html"
  ],
  "moduleFileExtensions": [
    "js",
    "json"
  ],
  "testMatch": [
    "**/*.test.js"
  ],
  "testPathIgnorePatterns": [
    "<rootDir>/ui_framework/dist/",
    "<rootDir>/ui_framework/doc_site/",
    "<rootDir>/ui_framework/generator-kui/"
  ],
  "transform": {
    "^.+\\.js$": "<rootDir>/src/jest/babelTransform.js"
  },
  "transformIgnorePatterns": [
    "[/\\\\]node_modules[/\\\\].+\\.js$"
  ],
  "snapshotSerializers": [
    "<rootDir>/node_modules/enzyme-to-json/serializer"
  ]
}

We've had some back and forth about the visuals of checkboxes, switches and radios. Some accept alternate list styling, some don't. Currently we use icons to designate "checked" states, but should probably incorporate color as well.

This is a minor issue for the moment, as we can change it at any time.

cc @cjcenizal, @formgeist, @Adrian-J, @gjones

This screenshot comes from Chrome 61. It looks kind of broken. We should figure out whether this is a build issue (@pugnascotia) or maybe an incomplete implementation (@snide)

Bottom bar uses fixed position on the screen. This means it will often overlap content at the bottom of the normally rendered page. We should find some way to apply a bottom padding (bottom bar height + $kuiSizeL) to the bottom of the page to account for it.

Possibilities I can think of:

• Do a domNode check and just apply a class to the body. Only one body in a page, so should be ok.
• Make some sort of prop on a body component to account for this kind of thing. Lower level item would need to talk up the chain.

There's only documentation for <KuiCheckboxGroup>.

Need to work with @gjones and @formgeist on a solution for subnav styling.

I suggest we just use AirBnB's eslint config. Kibana is gradually incorporating rules from their config until they're on par with one another, so we might as well just jump to the future.

The popover arrows need to be more aware of context menu title in regards to color. Right now they'll always appear dark no matter the position or usage. I have this mostly working for popovers themselves, just need to rework it for context menus.

• OK from Kevin
• Licensing (PR open at #20)
• Figure out eui.now.sh build automation (issue #21)

We're shadowing type='submit' for <button> because type is used to declare primary, secondary, and so on.

We should either rename type to something else such as hue='danger', or add a submit prop that results in <button type='submit'>.

Both approaches have their pros and cons. type fits the button "roles" very well, role would also clash with aria props, etc. At the same time, submit={ Boolean } wouldn't fix support for <button type='button'> either.