[UI Framework] Create Button React components in UI Framework. #10646

cjcenizal · 2017-03-02T03:45:08Z

To-do:

To-do in other PRs:

Add support for tooltip, possibly using CSS only (e.g. http://ubnt-css.herokuapp.com/#/app/tooltip)
Rename all .jsx files to .js

cjcenizal · 2017-03-02T19:17:44Z

I decided against creating a KuiSubmitButton for now. I'm not sure the abstraction adds enough value in exchange for the loss of flexibility:

const KuiSubmitButton = createButtonVariation({
  className: 'kuiButton--primary',
  isSubmit: true,
});

cjcenizal · 2017-03-21T01:26:39Z

@stacey-gammon @solostyle @kjbekkelund, could you take a look at this so far? Here are the questions I have top of mind:

How do you like the Button and ButtonIcon components? Are the examples in the doc site good enough? Are the React components themselves understandable? Is the relationship between Button and ButtonIcon 👍 ?
What do you think of the Jest test structure? I like using the describe blocks to group similar tests, and I like to test the interface of the component with different inputs. I'd really like us to all agree on a standard approach for writing tests, so that I can add a "how to" section to the README.
@kjbekkelund Does this look like I've configured Jest correctly? It's working but I'm not sure if I have anything silly leftover from the stuff you showed me.

Thanks, all.

cjcenizal · 2017-03-21T18:23:06Z

💯 ✋ 🎤 💥

kimjoar · 2017-03-21T20:29:36Z

ui_framework/components/button/button.js

+    type: props.isSubmit ? 'submit' : null,
+    disabled: props.isDisabled,
+    // onClick is optional, so don't even call it if it's not passed in.
+    onClick: props.onClick ? onClick : null,


This is a bit weird to me. It's checking props.onClick but passing in onClick. I see that thing is using props.onClick internally, but it makes this code less understandable. Maybe do the isDisabled check here, then if not disabled pass in onClick?

Also, as an api having onClick being potentially null is a bad api for underlying components. Better to give it a noop function? (e.g. () => {}, basically just an empty function that doens't do anything when called)

kimjoar · 2017-03-21T20:36:57Z

ui_framework/components/button/button.js

+      </span>
+    );
+
+  return React.createElement(elementType, {


Not a huge fan of createElement, if it can be easily avoided. Usually I'd prefer an if/else that either renders an input or an a, as it usually makes the code easier to read and the component more easily understood from where it's used.

If I wrote this I'd probably create a KibanaSubmitButton and a KibanaLinkButton or something like that. Might be too many cases I'm not thinking about though. It just feels like there will be less edge-casing by going that route. (e.g. could we make more params required if we did that? or maybe each implementation becomes simpler? (so instead of if-ing based on params internally, you choose at the call-site (the location in code where a function is called) which "function" to call (aka component to use))

kimjoar · 2017-03-21T20:38:25Z

I just took a quick look right now. I'll get back to this early tomorrow or a bit later today. Maybe it's easier to talk through this on zoom? (It's more "feeling" than "answers" from my part)

cjcenizal · 2017-03-22T00:15:16Z

Thanks @kjbekkelund. I implemented your suggestions. I wasn't sure how to reconcile the idea of creating SubmitButton and LinkButton with the requirements for PrimaryButton, DangerButton, etc. without creating a combinatorial explosion. So I just opted for the "early exit" route.

cjcenizal · 2017-03-22T04:55:22Z

@kjbekkelund I broke up KuiButton into KuiLinkButton and KuiSubmitButton, and added a type prop, similar to how @stacey-gammon was approaching components in a previous PR which explored converting the landing pages to React.

kimjoar · 2017-03-22T15:00:25Z

ui_framework/components/button/button.js

+import { KuiButtonIcon } from './button_icon/button_icon';
+
+const commonPropTypes = {
+  type: PropTypes.string,


I prefer not having this KuiButton.TYPE.BASIC thing, and rather just do

type: PropTypes.oneOf(['basic', 'hollow', 'danger', 'primary'])

I just think this reads so much better in code:

<KuiButton type='danger' />

It'll be as safe as the constant, plus if we for some reason want to rename things later on, that's super-simple to fix using jscodeshift.

fwiw I prefer using the enum style for these things, instead of the plain strings. I think it gives an obvious single location where all the types we handle are defined, and also indicates that the string is not meant for ui consumption.

I'm intrigued by the simplicity of Kim's suggestion. I'm going to give it a shot.

kimjoar · 2017-03-22T15:00:44Z

ui_framework/components/button/button.js

+    PropTypes.string,
+    PropTypes.array,
+    PropTypes.object,
+  ]),


This proptype is weird, it should just be className: PropTypes.string(?)

Yes, you're right.

kimjoar · 2017-03-22T15:01:09Z

ui_framework/components/button/button.js

+
+const commonPropTypes = {
+  type: PropTypes.string,
+  testSubject: PropTypes.string,


testSubject?

This value is assigned to a data-test-subj attribute on elements, which our functional tests use to find and interact with parts of the UI.

Hm, this could potentially be handled in the same way as I describe for "extra fields" (#10646 (comment)). Then you don't even have to include this here (I expect this is something we want to set in many places, and some of those places will be raw DOM elements, so there you'd have to write data-test-subj yourself instead of having it "in the abstraction)

Btw, from the React docs:

In React, all DOM properties and attributes (including event handlers) should be camelCased. For example, the HTML attribute tabindex corresponds to the attribute tabIndex in React. The exception is aria-* and data-* attributes, which should be lowercased.

(just to show that letting this stay as data-test-subj is okey from a React perspective)

kimjoar · 2017-03-22T15:04:18Z

ui_framework/components/button/button.js

+  icon: PropTypes.node,
+  isIconOnRight: PropTypes.bool,
+  children: PropTypes.node,
+  isLoading: PropTypes.bool,


Is the only reason onClick isn't required here that it's in commonPropTypes?

Why would we want to require onClick? I've become less and less restrictive in terms of setting props as required, because I've often run up against unanticipated edge cases. At this point, I prefer to avoid specifying props as required unless the component will actually break without them.

kimjoar · 2017-03-22T15:05:10Z

ui_framework/components/button/button.js

+  href: PropTypes.string,
+  target: PropTypes.string,
+  icon: PropTypes.node,
+  isIconOnRight: PropTypes.bool,


iconPlacement or iconLocation or something like that? Defaults to left, but can also be right.

kimjoar · 2017-03-22T15:18:22Z

ui_framework/components/button/button.js

+  return (
+    <a
+      href={props.href}
+      target={props.target}


What I usually do when there are extra fields like this is something like:

const { className, children, ...rest } = props const classes = classnames('my-class', className) return <a className={ classes } { ...rest }>{ children }</a>

That way I don't have to list all potential pure DOM apis that might be used, but I can still choose which ones I want to do something extra with (like className in the example).

That doesn't work with getCommonProps though.

Thanks for the tip! I'll keep it in mind.

kimjoar · 2017-03-22T15:22:14Z

ui_framework/components/button/button.js

+  );
+};
+
+KuiSubmitButton.propTypes = Object.assign({}, commonPropTypes, {


Any reason for using Object.assign instead of Object Rest/Spread Properties?

{ ...commonPropTypes, children: PropTypes.string }

@kjbekkelund weren't we talking in Tahoe how you didn't like using the spread property? Or am I thinking of something else? Or perhaps in this scenario, you can see it's benefit and don't believe in a hard rule?

Hm, might be I misunderstood something in some conversation. I love Object Rest/Spread and use them all the time.

(Really trying to think back, but can't remember. Too many conversations in Tahoe 🙈 It might have been in some other context that I just can't think off right now...)

It was at the end of the trip, we were eating dinner at that brewery place and talking with @simianhacker. It must have been something else that was brought up. I thought it was the spread operator (we were talking about destructing too, but you were in favor of that, Chris against if I remember correctly). Whatever was brought up, you and Chris were against it (I said I was for, but I guess I didn't know what was actually being talked about!). Well, glad we cleared that up!! Guess this is what I get for trying to 🍺 ,🍔 and follow 💬 at the same time. :)

Aaaaah, I remember the chat. I was pro desctructuring at the top (in most cases) and using that when passing to deeper components, while @simianhacker was pro keeping it as props.X when passing it down (even though that ended up being a mix of destructuring some things at the top and passing some in as props.X)

stacey-gammon · 2017-03-22T19:07:18Z

ui_framework/components/button/button.js

+    ) : (
+      <span>
+        {icon}
+        {wrappedChildren}


Do you think we should have a style rule for spaces inside the brackets? I previously created a rule in that style guide that said no spaces, but it was based on an invalid assumption (my bad). Just want that to be clear, so if we want to stick with the current eslint "object-curly-spacing" rule, we can add it to the react style guide. If you think a rule is too restrictive, I'm fine leaving it open.

I don't feel strongly about it one way or the other. Personally, I like no spaces when we're using brackets in the JSX context, but I like spaces when using brackets for objects. This helps me tell the two apart.

<Button prop1={someVariable} prop2={{ value1: 'value1', value2: 'value2' }} />

I ❤️ spaces within {} in JSX.

stacey-gammon · 2017-03-22T19:13:11Z

ui_framework/components/button/button.js

+  className: PropTypes.string,
+};
+
+const nonVoidPropTypes = {


I don't understanding the naming here. What makes these nonVoid and different from the props in common?

I added a comment to explain this.

cjcenizal · 2017-03-22T19:25:01Z

I implemented @kjbekkelund's suggestions regarding using string literals instead of enums for type. Personally, I really like this suggestion. The interface feels simpler and "lighter" to me. As Kim pointed out, we can use jscodeshift to refactor. What do you think, @stacey-gammon?

Also, right now I'm of the opinion that we should treat a component's interface as a layer on top of the native interface, i.e. native DOM properties and attributes. This means that someone using a component can't expect to be able to pass in any of these DOM properties and expect them to just work, unless the component explicitly supports them via propTypes and its implementation. I'm defaulting to this position because I prefer explicitness, and I just haven't seen a good example of why permitting DOM properties and attributes to pass-through is a good idea. I can be convinced otherwise, though!

stacey-gammon · 2017-03-22T19:32:00Z

I don't feel ultra strong (re: enums vs strings) in this particular situation, so SGTM.

I'm also willing to go with the flow for the native pass through props decision, but I think my preference is to allow pass through without being explicit. I was just thinking how often we are going to have to write that dataSubject propType eeeevvverywhere. And className. I was just thinking, there has to be a better way to do this without requiring it to be explicit on every single propType of every single component. 🤷‍♀️

kimjoar · 2017-03-22T19:43:59Z

+1 to what @stacey-gammon says. That's the reason I like passthrough: you don't have to wire up all the potential cases everywhere. E.g. why does target have to be specific? Also, I'm worried about testSubject: maybe it's only ever used for buttons, so it doesn't matter, but I'm just worried that we end up having code like this:

<div data-test-subj="foo">
  <KuiButton testSubject="bar" />
</div>

Which feels to me like it can cause confusion. (However, I haven't seen how it's used, so this is purely opinions from reading this code)

cjcenizal · 2017-03-22T19:49:48Z

These are really good points! I'm starting to come around. I'll try it out.

stacey-gammon · 2017-03-22T20:08:45Z

ui_framework/components/button/button.test.js

+            .toMatchSnapshot();
+        });
+      });
+    });


I find this to be really verbose, but I think that comes down to a stylistic difference. This is one more reason why enums for types would work well here. You could loop through the enum and make sure every option was rendered, so you couldn't accidentally add a string in that propType but forget to map it to a class name inside the button class.

something like this:

describe('all button types', () => { test('are rendered', () => { foreach (type in KuiButton.TYPES) { const $button = render(<KuiButton type="{ type }" />); expect($button).toMatchSnapshot(); } }); });

Then you never have to touch the tests for every new type added or removed, it'll all just work, and still all be tested.

barring the enum style, you could still do something like:

describe('all button types', () => { test('are rendered', () => { foreach (type in ['danger', 'basic', 'primary]) { const $button = render(<KuiButton type="{ type }" />); expect($button).toMatchSnapshot(); } }); });

But then still you have to remember to manually update the tests for every type added or removed, although you only have to add a single string, not a whole extra function. I know you get less information from the test output, but I think the benefit of automatically testing additional capabilities, as they are added to the code, is worth it.

One more thing... if you did add a string but forget to map it to a class name, I don't think that will throw an error here? Because it'll match the snapshot? unless we added a throw new Error('unhandled type') in the code? Or maybe jest tests will catch the invalid propType warning and treat that as a test failure?

stacey-gammon · 2017-03-22T20:11:45Z

ui_framework/components/button/button_group/button_group.js

+
+KuiButtonGroup.propTypes = {
+  children: PropTypes.node,
+  isUnited: PropTypes.bool,


What does isUnited represent?

stacey-gammon · 2017-03-22T20:14:37Z

ui_framework/components/button/button_icon/button_icon.js

+    'previous',
+    'next',
+    'loading',
+  ]),


Actually, if you just export this array, we can still programmatically test a button is rendered with every type in the array, without having to switch back to the enum notation (re: my earlier comment about automating the testing so it automatically tests every type, as they are added).

cjcenizal · 2017-03-22T22:25:45Z

@stacey-gammon @kjbekkelund I renamed testSubject to data-test-subj so we have consistency if people want to apply this attribute to both native components and custom components. But I couldn't figure out an elegant way to allow it to pass-through to the rendered element. Any suggestions?

kimjoar · 2017-03-23T02:56:20Z

I tried to write up some ideas a couple times, but my brain was borked so I had to code it up instead.

Check out idea.diff.txt

Right click, download, and git apply idea.diff.txt

Some tests fail because of shallow rendering. For Cloud UI we almost always fully render, and if needed, we some times mock out specific components (see e.g. https://github.com/elastic/cloud/blob/2a7897f950743cf292e43dc254a538fe4898d4c3/cloud-ui/public/components/Login/__tests__/Login.test.js and its snapshot). As I don't have a lot of experience using shallow testing in the context of snapshot tests I can't say much about whether it's a good approach or not at scale.

Some reasoning (hiiiighly subjective, so def feel free to skip this change ⚡️):

I'm using disabled instead of isDisabled for consistency with DOM attributes
I un-DRY-ed things a bit. It's still the same number of lines (actually a bit less, but might be because of some minor restructuring).
It feels easier for me to read (and that's probably just because it follows the flow I'm used too 🙈)
KuiLinkButton no longer accepts onClick — shouldn't the caller use KuiButton instead if that's needed?
onClick now expects the caller to handle data instead of injecting it. That also gives the caller the event, which seems like a positive step (e.g. if you need to preventDefault or stopPropagation)
All cases now accept arbitrary legal DOM attributes, and React will yell if they are incorrect, e.g.

Warning: Unknown prop isDisabled on tag.

I think my ideas came through in the code, but def ping me for a zoom if you want to chat about it.

stacey-gammon · 2017-03-23T12:44:28Z

@kjbekkelund any chance you can propose your changes as a PR? I can't get the diff to apply cleanly (though perhaps I am not going about it the right way - I either get errors or end up with a .rej. file which seems there is no easy way to selectively merge).

++ to getting rid of the data prop and just returning the event.

kimjoar · 2017-03-23T12:58:27Z

@stacey-gammon Good point. I created cjcenizal#17

…a flexbox wrapper. - Improve accessibility of kuiButtonIcon. - Remove disabled attribute from KuiLinkButton. - Remove kuiButton-isDisabled class from KuiButton and KuiSubmitButton.

…to expect kuiButton__inner.

kimjoar

This is growing quite large, so I think it's time to get it in. I like the current state of buttons and buttonGroup, so this LGTM 🚀

epixa · 2017-03-29T13:51:23Z

Something to think about for future PRs, for non-grunt tasks I'd recommend looking into the new scripts directory rather than burying them in the existing tasks directory: https://github.com/elastic/kibana/tree/master/scripts

stacey-gammon

LGTM as well. Funny how a PR can grow. Add a couple buttons and it turned into 62 files edited. Further proof an entire react landing page was a bit overly ambitious for a single PR! :)

kimjoar · 2017-03-29T14:05:19Z

@cjcenizal Maybe look at a follow-up that moves the Jest stuff into scripts? It would also be fantastic if we could get a setup that ran Jest from root, for all directories specified. That way we can specify that it should run for both ui framework and the new platform stuff I'm doing, plus it might make it simpler to adopt for other things. (so we don't end up with several different jest configs)

Maybe we can zoom about it? Just ping me on Slack

cjcenizal · 2017-03-29T15:31:57Z

jenkins, test this

cjcenizal · 2017-03-29T17:07:36Z

jenkins, test this

cjcenizal · 2017-03-29T17:54:49Z

jenkins, test this

kimjoar · 2017-03-29T20:15:27Z

tasks/utils/ui_framework_test_config.js

+  moduleNameMapper: {
+    '^.+\\.(jpg|jpeg|png|gif|eot|otf|webp|svg|ttf|woff|woff2|mp4|webm)$': resolve('config/jest/FileStub.js'),
+    '^.+\\.css$': resolve('config/jest/CSSStub.js'),
+    '^.+\\.scss$': resolve('config/jest/CSSStub.js')


I think these files might be missing? Probably not importing any of them, so don't hit this problem in this PR. We can just remove this from here and add it later? (Just to get this PR in)

cjcenizal · 2017-03-29T20:17:33Z

Note: the tests finally passed.

epixa · 2017-03-29T20:25:50Z

@cjcenizal Were you getting flaky failures? What were their nature? I'm not aware of any timing issues in the tests at the moment.

Backports PR #10646 **Commit 1:** Create Button React components in UI Framework. * Original sha: 68e14a2 * Authored by CJ Cenizal <cj@cenizal.com> on 2017-03-02T03:43:14Z **Commit 2:** Integrate button icon variations into button_icon.js. Integrate button variations inbot button.js. - Rename classes prop to className. * Original sha: efa0a26 * Authored by CJ Cenizal <cj@cenizal.com> on 2017-03-02T17:26:16Z **Commit 3:** Add KuiLoadingButtonIcon and isLoading prop for KuiButton. * Original sha: c55ff13 * Authored by CJ Cenizal <cj@cenizal.com> on 2017-03-02T19:11:52Z **Commit 4:** Add Jest test coverage for UI Framework. - Add tests for KuiButton. - Generate report in ui_framework/jest/report. * Original sha: b461275 * Authored by CJ Cenizal <cj@cenizal.com> on 2017-03-21T01:17:29Z **Commit 5:** Add tests for KuiButtonIcon and KuiButtonGroup. * Original sha: 933f8ec * Authored by CJ Cenizal <cj@cenizal.com> on 2017-03-21T18:20:57Z **Commit 6:** Add both React and HTML examples for KuiButton. * Original sha: 36e0ea6 * Authored by CJ Cenizal <cj@cenizal.com> on 2017-03-21T20:52:04Z **Commit 7:** Update UI Framework README with instructions on creating and testing React components. * Original sha: 802f1ea * Authored by CJ Cenizal <cj@cenizal.com> on 2017-03-21T23:52:29Z **Commit 8:** Move KuiButton isDisabled check from onClick handler to prop assginment. * Original sha: 0132a55 * Authored by CJ Cenizal <cj@cenizal.com> on 2017-03-21T23:58:34Z **Commit 9:** Refactor kuiButton to not use createElement, and instead exit early and return the correct element. * Original sha: 31219b3 * Authored by CJ Cenizal <cj@cenizal.com> on 2017-03-22T00:10:03Z **Commit 10:** Redesign KuiButton and KuiButtonIcon to accept a type prop. * Original sha: a8000c9 * Authored by CJ Cenizal <cj@cenizal.com> on 2017-03-22T04:13:29Z **Commit 11:** Break KuiButton apart into KuiButton, KuiLinkButton, and KuiSubmitButton. * Original sha: 020d94c * Authored by CJ Cenizal <cj@cenizal.com> on 2017-03-22T04:53:36Z **Commit 12:** Move KuiButtonIcon and KuiButtonGroup into their own directories. * Original sha: 119abf5 * Authored by CJ Cenizal <cj@cenizal.com> on 2017-03-22T05:01:59Z **Commit 13:** Remove unused icon var from KuiSubmitButton. * Original sha: 24a9a4f * Authored by CJ Cenizal <cj@cenizal.com> on 2017-03-22T05:05:32Z **Commit 14:** Use simpler rest parameter syntax instead of Object.assign for defining KuiButton propTypes. * Original sha: 4119bfb * Authored by CJ Cenizal <cj@cenizal.com> on 2017-03-22T18:42:31Z **Commit 15:** Refactor KuiButton and KuiButtonIcon type prop to emphasize passing string literals instead of enums. * Original sha: d50d081 * Authored by CJ Cenizal <cj@cenizal.com> on 2017-03-22T19:06:42Z **Commit 16:** Add comment to explain role of nonVoidPropTypes in KuiButton. * Original sha: f9c55fa * Authored by CJ Cenizal <cj@cenizal.com> on 2017-03-22T19:15:57Z **Commit 17:** Dynamically define KuiButton and KuiButtonIcon tests for type prop. * Original sha: 0c5d2fd * Authored by CJ Cenizal <cj@cenizal.com> on 2017-03-22T20:46:17Z **Commit 18:** Fix Jest coverage configuration for deeply-nested dirs. * Original sha: 6d724fc * Authored by CJ Cenizal <cj@cenizal.com> on 2017-03-22T21:27:30Z **Commit 19:** Rename prop testSubject to data-test-subj. * Original sha: a9a4652 * Authored by CJ Cenizal <cj@cenizal.com> on 2017-03-22T22:22:51Z **Commit 20:** button idea * Original sha: d1eed94 * Authored by Kim Joar Bekkelund <kjbekkelund@gmail.com> on 2017-03-23T12:57:03Z * Committed by CJ Cenizal <cj@cenizal.com> on 2017-03-28T15:21:03Z **Commit 21:** Remove unnecessary onClick mentions * Original sha: a2d045e * Authored by Kim Joar Bekkelund <kjbekkelund@gmail.com> on 2017-03-23T15:36:34Z * Committed by CJ Cenizal <cj@cenizal.com> on 2017-03-28T15:21:03Z **Commit 22:** - Update KuiLinkButton to preventDefault on click when disabled. - Update getClassName helper to accommodate loading icons. - Update tests. * Original sha: 4016eba * Authored by CJ Cenizal <cj@cenizal.com> on 2017-03-24T00:45:00Z **Commit 23:** Update tests with HTML attributes group. Add test for aria-label. * Original sha: 05d934b * Authored by CJ Cenizal <cj@cenizal.com> on 2017-03-24T17:08:29Z **Commit 24:** Add UI Framework to linting task. * Original sha: 3bc5016 * Authored by CJ Cenizal <cj@cenizal.com> on 2017-03-24T17:24:41Z **Commit 25:** Refactor HTML attribute tests to be more succinct. * Original sha: 322a968 * Authored by CJ Cenizal <cj@cenizal.com> on 2017-03-24T18:58:45Z **Commit 26:** Remove backticks from ui_framework_test task. * Original sha: 2187260 * Authored by CJ Cenizal <cj@cenizal.com> on 2017-03-24T19:00:24Z **Commit 27:** Add eslintrc file to ui_framework, for Jest-specific rules. * Original sha: 94daf03 * Authored by CJ Cenizal <cj@cenizal.com> on 2017-03-24T19:06:42Z **Commit 28:** Add UI Framework Jest tests to npm test script. Create separate scripts for watching and generating coverage reports. * Original sha: 2fa3cf7 * Authored by CJ Cenizal <cj@cenizal.com> on 2017-03-24T20:58:00Z **Commit 29:** Remove redundant kuiSubmitButton tests. * Original sha: 8d4222a * Authored by CJ Cenizal <cj@cenizal.com> on 2017-03-24T21:06:13Z **Commit 30:** Document Enzyme-specific Webpack configuration. * Original sha: ad9616f * Authored by CJ Cenizal <cj@cenizal.com> on 2017-03-24T21:09:07Z

…ic#10646) * Create KuiButton, KuiLinkButton, KuiSubmitButton, and KuiButtonIcon React components in UI Framework. * Add Jest test coverage for UI Framework, generate report in ui_framework/jest/report. * Add UI Framework to linting task. * Update UI Framework README with instructions on creating and testing React components. * Add both React and HTML examples. * Add UI Framework Jest tests to npm test script. Create separate scripts for watching and generating coverage reports. * Fix appearance of kuiButtons with icons throughout Kibana, by adding a flexbox wrapper. * Improve accessibility of kuiButtonIcon. * Remove disabled attribute from KuiLinkButton. * Remove kuiButton-isDisabled class from KuiButton and KuiSubmitButton.

… (#10941) * Create KuiButton, KuiLinkButton, KuiSubmitButton, and KuiButtonIcon React components in UI Framework. * Add Jest test coverage for UI Framework, generate report in ui_framework/jest/report. * Add UI Framework to linting task. * Update UI Framework README with instructions on creating and testing React components. * Add both React and HTML examples. * Add UI Framework Jest tests to npm test script. Create separate scripts for watching and generating coverage reports. * Fix appearance of kuiButtons with icons throughout Kibana, by adding a flexbox wrapper. * Improve accessibility of kuiButtonIcon. * Remove disabled attribute from KuiLinkButton. * Remove kuiButton-isDisabled class from KuiButton and KuiSubmitButton.

cjcenizal added the Team:Platform-Design Team Label for Kibana Design Team. Support the Analyze group of plugins. label Mar 2, 2017

cjcenizal requested a review from stacey-gammon March 2, 2017 17:41

cjcenizal force-pushed the feature/react-commponents-ui-framework branch from 912504f to 89f0c3f Compare March 21, 2017 01:18

cjcenizal requested a review from kimjoar March 21, 2017 01:21

kimjoar reviewed Mar 21, 2017

View reviewed changes

cjcenizal force-pushed the feature/react-commponents-ui-framework branch from c1dcca8 to d2e16a6 Compare March 22, 2017 00:13

cjcenizal force-pushed the feature/react-commponents-ui-framework branch from 31f617f to 3a11ef3 Compare March 22, 2017 04:57

kimjoar reviewed Mar 22, 2017

View reviewed changes

stacey-gammon reviewed Mar 22, 2017

View reviewed changes

cjcenizal force-pushed the feature/react-commponents-ui-framework branch from 752e861 to 61478c7 Compare March 22, 2017 19:07

stacey-gammon reviewed Mar 22, 2017

View reviewed changes

cjcenizal force-pushed the feature/react-commponents-ui-framework branch from 47bf223 to a7d468b Compare March 22, 2017 22:14

cjcenizal added 3 commits March 28, 2017 08:21

Remove redundant kuiSubmitButton tests.

8d4222a

Document Enzyme-specific Webpack configuration.

ad9616f

Fix appearance of kuiButtons with icons throughout Kibana, by adding …

c28bbd7

…a flexbox wrapper. - Improve accessibility of kuiButtonIcon. - Remove disabled attribute from KuiLinkButton. - Remove kuiButton-isDisabled class from KuiButton and KuiSubmitButton.

cjcenizal force-pushed the feature/react-commponents-ui-framework branch from 3be3b7b to c28bbd7 Compare March 28, 2017 15:21

Update JSX to not enclose prop arguments in whitespace. Update tests …

17e0838

…to expect kuiButton__inner.

cjcenizal force-pushed the feature/react-commponents-ui-framework branch from 6e34ff3 to 17e0838 Compare March 28, 2017 17:41

kimjoar approved these changes Mar 29, 2017

View reviewed changes

stacey-gammon approved these changes Mar 29, 2017

View reviewed changes

kimjoar reviewed Mar 29, 2017

View reviewed changes

Remove unused Jest stub files.

98e1e38

cjcenizal merged commit b604911 into elastic:master Mar 29, 2017

cjcenizal deleted the feature/react-commponents-ui-framework branch March 29, 2017 20:25

elastic-jasper mentioned this pull request Mar 29, 2017

[5.x] [UI Framework] Create Button React components in UI Framework. #10939

Closed

elastic-jasper mentioned this pull request Mar 29, 2017

[5.x] [UI Framework] Create Button React components in UI Framework. #10940

Closed

cjcenizal mentioned this pull request Mar 29, 2017

[5.x] [UI Framework] Create Button React components in UI Framework. (#10646) #10941

Merged

cjcenizal mentioned this pull request Apr 4, 2017

React search box tool bar #10821

Merged

cjcenizal mentioned this pull request May 1, 2017

UiFramework: Modals in react #11500

Merged

[UI Framework] Create Button React components in UI Framework. #10646

[UI Framework] Create Button React components in UI Framework. #10646

Conversation

cjcenizal commented Mar 2, 2017 • edited Loading

cjcenizal commented Mar 2, 2017

cjcenizal commented Mar 21, 2017

cjcenizal commented Mar 21, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kimjoar commented Mar 21, 2017 • edited Loading

cjcenizal commented Mar 22, 2017 • edited Loading

cjcenizal commented Mar 22, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cjcenizal Mar 22, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cjcenizal commented Mar 22, 2017

stacey-gammon commented Mar 22, 2017

kimjoar commented Mar 22, 2017

cjcenizal commented Mar 22, 2017

stacey-gammon Mar 22, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cjcenizal commented Mar 22, 2017

kimjoar commented Mar 23, 2017 • edited Loading

stacey-gammon commented Mar 23, 2017

kimjoar commented Mar 23, 2017

kimjoar left a comment

Choose a reason for hiding this comment

epixa commented Mar 29, 2017

stacey-gammon left a comment

Choose a reason for hiding this comment

kimjoar commented Mar 29, 2017

cjcenizal commented Mar 29, 2017

cjcenizal commented Mar 29, 2017

cjcenizal commented Mar 29, 2017

Choose a reason for hiding this comment

cjcenizal commented Mar 29, 2017

epixa commented Mar 29, 2017

cjcenizal commented Mar 2, 2017 •

edited

Loading

cjcenizal commented Mar 21, 2017 •

edited

Loading

kimjoar commented Mar 21, 2017 •

edited

Loading

cjcenizal commented Mar 22, 2017 •

edited

Loading

cjcenizal Mar 22, 2017 •

edited

Loading

stacey-gammon Mar 22, 2017 •

edited

Loading

kimjoar commented Mar 23, 2017 •

edited

Loading