[UI Framework] Reorganize UI Framework and add Yeoman generator #13172

cjcenizal · 2017-07-28T05:18:56Z

This cherry-picks multiple commits from the k7-ui-framework branch, squashing similar ones together:

weltenwort

I cannot comment on the efficacy of the scss reorganization, but the rest looks quite well-organized to me. I left a few small inline comments.

The generators are very handy! They could be made even more efficient for regular use by also adding the prompt choices as command-line arguments. I would be happy to create a PR for that.

weltenwort · 2017-08-08T08:06:44Z

package.json

-    "uiFramework:build": "grunt uiFramework:build"
+    "uiFramework:build": "grunt uiFramework:build",
+    "uiFramework:create": "yo ./ui_framework/generator-kui/app/component.js",
+    "uiFramework:document": "yo ./ui_framework/generator-kui/app/documentation.js"


Maybe these could be qualified as createComponent and documentComponent to indicate what will be created or documented?

Also, I wonder if #11095 is relevant here. @spalger, any thoughts?

weltenwort · 2017-08-08T08:17:39Z

ui_framework/README.md

@@ -54,15 +93,25 @@ you created.

 This makes your styles available to Kibana and the UI Framework documentation.

-### Create the React component
+#### Create the React component

 1. Create the React component(s) in the same directory as the related SCSS file(s).
 2. Export these components from an `index.js` file.
 3. Re-export these components from `ui_framework/components/index.js`.


I think this path has changed to ui_framework/src/components/index.js.

weltenwort · 2017-08-08T08:19:09Z

ui_framework/README.md

-  KuiButtonIcon,
-} from '../path/to/ui_framework/components';
-```
-
 ## Creating components

 There are four steps to creating a new component:

 1. Create the SCSS for the component in `ui_framework/components`.


I think this path has changed to ui_framework/src/components.

There's a src/components.js file and a src/services.js file which re-export the relevant components and services, so I think we can leave these docs as-is. I created this file to avoid having to make any changes in the way the UI Framework is consumed by Kibana.

Ah ignore that comment! I wasn't paying close enough attention. You're right, I'll fix this.

weltenwort · 2017-08-08T08:19:31Z

ui_framework/README.md

+
+### Manually
+
+#### Create component SCSS

 1. Create a directory for your component in `ui_framework/components`.


I think this path has changed to ui_framework/src/components.

weltenwort · 2017-08-08T08:28:16Z

src/jest/config.json

+    "!ui_framework/src/services/**/*/index.js",
+    "ui_framework/src/components/**/*.js",
+    "!ui_framework/src/components/index.js",
+    "!ui_framework/src/components/**/*/index.js"
  ],
  "moduleNameMapper": {
    "^ui_framework/components": "<rootDir>/ui_framework/components",


I think this path has changed to ui_framework/src/components.

Actually, I think this one is OK, because there's a ui_framework/components/index.js file which re-exports everything from src.

weltenwort · 2017-08-08T09:49:59Z

ui_framework/generator-kui/component/templates/component.js

@@ -0,0 +1,33 @@
+import React, {
+  Component,
+  PropTypes,


In the interest of forward-compatibility we might want to take PropTypes from the prop-types package:

import PropTypes from 'prop-types';

weltenwort · 2017-08-08T09:50:48Z

ui_framework/generator-kui/component/templates/stateless_function.js

@@ -0,0 +1,20 @@
+import React, {
+  PropTypes,


See the comment about prop-types above.

weltenwort · 2017-08-08T09:51:17Z

ui_framework/generator-kui/component/templates/stateless_function.js

+};
+
+<%= componentName %>.propTypes = {
+};


How about adding className and children by default here as well?

weltenwort · 2017-08-08T10:22:11Z

ui_framework/generator-kui/documentation/index.js

+        name: 'folderName',
+        type: 'input',
+        store: true,
+      });


Can we make this default to the component name?

prompts.push({ // ... default: (answers) => answers.name, // ... });

weltenwort · 2017-08-08T10:30:00Z

ui_framework/generator-kui/documentation/index.js

+      this.log(chalk.white('\n// Import route definition into routes.js.'));
+      this.log(
+        `{\n` +
+        `  name: ${chalk.cyan(`'${componentExampleName}'`)},\n` +


The proposed route name collides with that of the component docs. If the user just copy+pastes this the sandbox menu entry will link to the component doc page. Maybe we could propose something with a prefix or suffix to avoid that?

weltenwort · 2017-08-08T10:47:13Z

The test failure looks genuine. The yo templates need to be excluded from the test runner patterns.

w33ble · 2017-08-08T17:10:54Z

My dislike of yoeman aside, is there a benefit to having the generator in the ui-framework, and not just as another tool in packages? Don't you have to install the generator on its own anyway?

cjcenizal · 2017-08-08T22:41:59Z

package.json

-    "uiFramework:build": "grunt uiFramework:build"
+    "uiFramework:build": "grunt uiFramework:build",
+    "uiFramework:create": "./node_modules/_bin/yo ./ui_framework/generator-kui/app/component.js",
+    "uiFramework:document": "./node_modules/_bin/yo ./ui_framework/generator-kui/app/documentation.js"


@jbudz Will these paths be a problem on Windows machines?

because of the slashes?

You for sure do not need the whole node_modules part, you can just use yo. The beauty of npm scripts is that all the modules are injected into the $PATH.

I can't speak to the path following the command though.

cjcenizal · 2017-08-08T22:44:30Z

@w33ble I set up the generator and the tasks which run it so that you don't need to install anything except the regular NPM dependencies via npm i. I think the benefit of having the generator inside the UI Framework is that it's very closely related to the UI Framework. Though it can be used to create components outside of it, it's optimized for creating components and documentation within it.

w33ble · 2017-08-08T23:37:24Z

ui_framework/README.md

+#### Install Yeoman
+
+```bash
+npm install -g yo


If all of this runs from npm scripts, is this required?

Good catch!

cjcenizal · 2017-08-09T00:36:45Z

@w33ble @weltenwort Thanks for the feedback! Ready for another review.

cjcenizal · 2017-08-09T00:37:18Z

The generators are very handy! They could be made even more efficient for regular use by also adding the prompt choices as command-line arguments. I would be happy to create a PR for that.

@weltenwort That would be great, thank you.

weltenwort · 2017-08-11T08:15:05Z

jenkins, test this

…ts directory. (elastic#12809) - Remove global styles, e.g. body and html element selectors.

…c directory. (elastic#12880) - Add components/index.js and services/index.js files to continue to export JS modules from the root.

* Support creation of components. * Add documentation generator for main page, demo, and sandbox. Add additional documentation snippets to KUI generator. (elastic#13076) Fix incorrect use of double quotes in KUI generator snippet. (elastic#13086) Remove infrequently used imports from the KUI generator test template. (elastic#13110)

elastic#13102)

…emplate. (elastic#13377)

- Add dashboard back to Jest config. - Add missing form and tool_bar variables.

…m README.

…tic#13172) * Reorganize documentation styles so they all live in doc_site/components directory. (elastic#12809) - Remove global styles, e.g. body and html element selectors. * Create global_styles dir with sub-directories. (elastic#12833) * Add SCSS style guide. (elastic#12850) * Refactor UI Framework directory structure to house everything in a src directory. (elastic#12880) - Add components/index.js and services/index.js files to continue to export JS modules from the root. * Add KUI Yeoman generator. * Support creation of components. * Add documentation generator for main page, demo, and sandbox. - Add additional documentation snippets to KUI generator. (elastic#13076) - Fix incorrect use of double quotes in KUI generator snippet. (elastic#13086) - Remove infrequently used imports from the KUI generator test template. (elastic#13110) * Mock assets files for Jest. (elastic#13060) * Fix broken coverage report paths in Jest config. (elastic#13082) * Update eslint config to lint the new UI Framework directory structure. (elastic#13102) * Fix positioning of doc site pagination buttons. (elastic#13203) * Support hasReact prop for sandboxes. (elastic#13270) * Remove deprecated used of component mixin from KUI generator's SCSS template. (elastic#13377) * Fix rebasing errors. - Add dashboard back to Jest config. - Add missing form and tool_bar variables. * Rename tasks to createComponent and documentComponent. * Reference correct src paths in README. * Add children and className to templates' propTypes. * Add default folder name for page demo. * Add suffix to sandbox routes. * Specify testPathIgnorePatterns more clearly. * Rename component.test.js to test.js so that Jenkins won't try to run it. * Update npm scripts to depend on local yo dependency, not global. * Add ui_framework/src to copy task. * Simplify npm scripts and remove requirement for installing Yeoman from README. * Add services to moduleNameMapper in jest config. * Clean up Button and Gallery examples.

…) (#13477) * Reorganize documentation styles so they all live in doc_site/components directory. (#12809) - Remove global styles, e.g. body and html element selectors. * Create global_styles dir with sub-directories. (#12833) * Add SCSS style guide. (#12850) * Refactor UI Framework directory structure to house everything in a src directory. (#12880) - Add components/index.js and services/index.js files to continue to export JS modules from the root. * Add KUI Yeoman generator. * Support creation of components. * Add documentation generator for main page, demo, and sandbox. - Add additional documentation snippets to KUI generator. (#13076) - Fix incorrect use of double quotes in KUI generator snippet. (#13086) - Remove infrequently used imports from the KUI generator test template. (#13110) * Mock assets files for Jest. (#13060) * Fix broken coverage report paths in Jest config. (#13082) * Update eslint config to lint the new UI Framework directory structure. (#13102) * Fix positioning of doc site pagination buttons. (#13203) * Support hasReact prop for sandboxes. (#13270) * Remove deprecated used of component mixin from KUI generator's SCSS template. (#13377) * Fix rebasing errors. - Add dashboard back to Jest config. - Add missing form and tool_bar variables. * Rename tasks to createComponent and documentComponent. * Reference correct src paths in README. * Add children and className to templates' propTypes. * Add default folder name for page demo. * Add suffix to sandbox routes. * Specify testPathIgnorePatterns more clearly. * Rename component.test.js to test.js so that Jenkins won't try to run it. * Update npm scripts to depend on local yo dependency, not global. * Add ui_framework/src to copy task. * Simplify npm scripts and remove requirement for installing Yeoman from README. * Add services to moduleNameMapper in jest config. * Clean up Button and Gallery examples.

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

cjcenizal requested review from kimjoar and snide July 28, 2017 05:20

kimjoar removed their request for review July 30, 2017 21:19

cjcenizal requested a review from weltenwort August 3, 2017 15:01

cjcenizal force-pushed the improvement/ui-framework-organization branch from 328f389 to 2bc7291 Compare August 7, 2017 22:44

weltenwort reviewed Aug 8, 2017

View reviewed changes

cjcenizal mentioned this pull request Aug 8, 2017

[UI Framework] [K7] Refine generator-kui. #13400

Closed

cjcenizal force-pushed the improvement/ui-framework-organization branch from f6a7e34 to 72e8e10 Compare August 8, 2017 22:40

cjcenizal commented Aug 8, 2017

View reviewed changes

w33ble reviewed Aug 8, 2017

View reviewed changes

cjcenizal force-pushed the improvement/ui-framework-organization branch from df69cbf to b0600ea Compare August 10, 2017 23:25

weltenwort approved these changes Aug 11, 2017

View reviewed changes

cjcenizal added 11 commits August 11, 2017 08:41

Reorganize documentation styles so they all live in doc_site/componen…

3ae1af3

…ts directory. (elastic#12809) - Remove global styles, e.g. body and html element selectors.

Create global_styles dir with sub-directories. (elastic#12833)

0f3c8aa

Add SCSS style guide. (elastic#12850)

528ae68

Refactor UI Framework directory structure to house everything in a sr…

d9a48c6

…c directory. (elastic#12880) - Add components/index.js and services/index.js files to continue to export JS modules from the root.

Mock assets files for Jest. (elastic#13060)

77c9fe2

Fix broken coverage report paths in Jest config. (elastic#13082)

1abbced

Update eslint config to lint the new UI Framework directory structure. (

e170437

elastic#13102)

Remove deprecated used of component mixin from KUI generator's SCSS t…

c0fdfa7

…emplate. (elastic#13377)

Support hasReact prop for sandboxes. (elastic#13270)

cbe1de8

Fix rebasing errors.

1a44d40

- Add dashboard back to Jest config. - Add missing form and tool_bar variables.

cjcenizal added 13 commits August 11, 2017 08:41

Fix positioning of doc site pagination buttons. (elastic#13203)

3d8a442

Specify testPathIgnorePatterns more clearly.

740060f

Rename component.test.js to test.js so that Jenkins won't try to run it.

a84d68f

Update npm scripts to depend on local yo dependency, not global.

856bdf0

Add ui_framework/src to copy task.

786261c

Simplify npm scripts and remove requirement for installing Yeoman fro…

5ab9203

…m README.

Rename tasks to createComponent and documentComponent.

07dc0eb

Reference correct src paths in README.

2751a84

Add children and className to templates' propTypes.

a98b631

Add default folder name for page demo.

201ed91

Add suffix to sandbox routes.

621b235

Add services to moduleNameMapper in jest config.

96528fd

Clean up Button and Gallery examples.

8f2b43a

cjcenizal force-pushed the improvement/ui-framework-organization branch from b0600ea to 8f2b43a Compare August 11, 2017 15:42

cjcenizal merged commit 720297d into elastic:master Aug 11, 2017

cjcenizal deleted the improvement/ui-framework-organization branch August 11, 2017 15:48

cjcenizal added v6.1.0 v7.0.0 labels Aug 11, 2017

cjcenizal mentioned this pull request Aug 11, 2017

[6.x] [UI Framework] Reorganize UI Framework and add Yeoman generator (#13172) #13477

Merged

cjcenizal mentioned this pull request Aug 11, 2017

[UI Framework] [K7] Cherry-pick generator and doc site improvements #13479

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[UI Framework] Reorganize UI Framework and add Yeoman generator #13172

[UI Framework] Reorganize UI Framework and add Yeoman generator #13172

cjcenizal commented Jul 28, 2017 •

edited

Loading

weltenwort left a comment

weltenwort Aug 8, 2017

weltenwort Aug 8, 2017

weltenwort Aug 8, 2017

cjcenizal Aug 8, 2017

cjcenizal Aug 8, 2017

weltenwort Aug 8, 2017

weltenwort Aug 8, 2017

cjcenizal Aug 9, 2017 •

edited

Loading

weltenwort Aug 8, 2017

weltenwort Aug 8, 2017

weltenwort Aug 8, 2017

weltenwort Aug 8, 2017

weltenwort Aug 8, 2017

weltenwort commented Aug 8, 2017

w33ble commented Aug 8, 2017

cjcenizal Aug 8, 2017

rhoboat Aug 8, 2017

cjcenizal Aug 8, 2017

w33ble Aug 8, 2017

cjcenizal commented Aug 8, 2017

w33ble Aug 8, 2017

cjcenizal Aug 8, 2017

cjcenizal commented Aug 9, 2017

cjcenizal commented Aug 9, 2017

weltenwort commented Aug 11, 2017

[UI Framework] Reorganize UI Framework and add Yeoman generator #13172

[UI Framework] Reorganize UI Framework and add Yeoman generator #13172

Conversation

cjcenizal commented Jul 28, 2017 • edited Loading

weltenwort left a 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 Aug 9, 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weltenwort commented Aug 8, 2017

w33ble commented Aug 8, 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

cjcenizal commented Aug 8, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cjcenizal commented Aug 9, 2017

cjcenizal commented Aug 9, 2017

weltenwort commented Aug 11, 2017

cjcenizal commented Jul 28, 2017 •

edited

Loading

cjcenizal Aug 9, 2017 •

edited

Loading