App mgmt pages #552

liiil825 · 2018-11-26T10:15:35Z

No description provided.

codecov · 2018-11-26T10:17:42Z

Codecov Report

Merging #552 into master will increase coverage by 0.06%.
The diff coverage is 16.12%.

@@            Coverage Diff             @@
##           master     #552      +/-   ##
==========================================
+ Coverage   16.17%   16.23%   +0.06%     
==========================================
  Files          96       98       +2     
  Lines        1533     1595      +62     
  Branches      327      341      +14     
==========================================
+ Hits          248      259      +11     
- Misses       1230     1281      +51     
  Partials       55       55

Impacted Files	Coverage Δ
src/components/Base/Upload/index.jsx	`3.5% <ø> (ø)`	⬆️
src/components/Layout/index.js	`0% <ø> (ø)`	⬆️
src/utils/icons.js	`100% <ø> (ø)`	⬆️
src/components/Layout/index.jsx	`0% <ø> (ø)`	⬆️
src/components/Layout/Stepper/index.jsx	`0% <0%> (ø)`
src/components/Base/DocLink/index.jsx	`71.42% <71.42%> (ø)`
src/utils/sock-client.js	`0% <0%> (ø)`	⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 10c9ef3...720b331. Read the comment docs.

src/components/Base/index.js

src/components/Base/Link/index.jsx

liiil825 · 2018-12-05T02:28:13Z

#516

sunnywx · 2018-12-05T02:58:20Z

src/components/Base/Input/input.jsx

+    const value = target.type === 'checkbox' ? target.checked : target.value;
+    const name = target.name;
+
+    this.props.valueChange(name, value);


valueChange is redundant, you only need onChange

I want a function pass name and it do not affect before

sunnywx · 2018-12-05T02:59:08Z

src/components/Base/Input/input.jsx

+    const name = target.name;
+
+    this.props.valueChange(name, value);
+    this.props.onChange(event);


since you get target value, using this.props.onChange(value) is better

@action valueChange = (name, value) => { this.attribute[name] = value; if (this.attribute.name) { this.disableNextStep = false; } else { this.disableNextStep = true; } this.errorMessage = ''; };

valueChange pass the name param, I can use a generic function to change the store value

sunnywx · 2018-12-05T03:01:38Z

src/components/Base/Link/index.jsx

+import links from 'config/doc-link';
+
+@translate()
+export default class DocLink extends Component {


Since the component name is DocLink, you should name this file name as DocLink too. Also the name is conflict with react-dom Link

sunnywx · 2018-12-05T03:07:38Z

src/components/Base/Link/index.jsx

+    }
+    if (isExternal) {
+      if (!linkTo) {
+        console.error(


If no need render, just throw Error

sunnywx · 2018-12-05T03:10:16Z

src/components/Base/Link/index.jsx

+    const {
+      t, to, name, children, className, isExternal
+    } = this.props;
+    let text = t(`LINK_${name}`);


No need to add t() here, since you compare text with LINK_${name}

I want a default text from it props of name to t(LINK_${name})

sunnywx · 2018-12-05T03:12:39Z

src/components/Base/Upload/index.jsx

@@ -170,10 +170,9 @@ export default class Upload extends Component {
    return (
      <span
        {...events}
-        className={classNames('upload', {
+        className={classNames(className, 'upload', {


className placed at last is better, since it's a override style

sunnywx · 2018-12-05T03:13:29Z

src/components/InfiniteScroll/InfiniteScroll.jsx

@@ -0,0 +1,244 @@
+import React, { Component } from 'react';


filename is wrong, just rename it as InfiniteScroll/index.jsx

sunnywx · 2018-12-05T03:14:54Z

src/components/InfiniteScroll/InfiniteScroll.jsx

+
+export default class InfiniteScroll extends Component {
+  static propTypes = {
+    children: PropTypes.node.isRequired,


No need to add children here

sunnywx · 2018-12-05T06:08:55Z

src/components/Layout/Stepper/index.jsx

+    children: PropTypes.node,
+    className: PropTypes.string,
+    name: PropTypes.string,
+    store: PropTypes.shape({


The name store is not good, rename it as stepOption

sunnywx · 2018-12-05T06:10:33Z

src/components/Layout/Stepper/index.jsx

+    const { steps, activeStep } = store;
+    const width = `${activeStep * 100 / steps}%`;
+    let className = 'headerStepNotFinished';
+    if (activeStep > steps) {


refactor it to

const className = activeStep > step ? '..' : '..'

sunnywx · 2018-12-05T06:13:34Z

src/components/Layout/Stepper/index.jsx

+      return null;
+    }
+
+    const NAME = `STEPPER_FOOTER_${name.toLocaleUpperCase()}_${activeStep}`;


Don't set variable name like this

sunnywx · 2018-12-05T06:14:29Z

src/components/Layout/index.js

@@ -12,5 +12,6 @@ export Card from './Card';

 export SideNav from './SideNav';
 export BreadCrumb from './BreadCrumb';
+export LayoutStep from './Stepper';


Using name Stepper is ok

sunnywx · 2018-12-05T06:39:13Z

src/components/Layout/index.jsx

      pageTitle
    } = this.props;

    const { isNormal, isDev, isAdmin } = this.props.user;
    const hasMenu = (isDev || isAdmin) && !isHome;
    const paths = ['/dashboard', '/profile', '/ssh_keys', '/dev/apps'];
    const isCenterPage = Boolean(pageTitle); // detail page, only one level menu
-    const hasSubNav = hasMenu && !isCenterPage && !paths.includes(match.path);
+    const hasSubNav = !noSubMenu && hasMenu && !isCenterPage && !paths.includes(match.path);


hasMenu should check before noSubMenu:

hasMenu && !noSubMenu

sunnywx · 2018-12-05T06:39:50Z

src/components/Loading/Page/index.jsx

@@ -0,0 +1,37 @@
+import React, { Component, Fragment } from 'react';


This file can be removed, duplicate with Loading/index.jsx

sunnywx · 2018-12-05T06:42:04Z

src/config/doc-link.js

@@ -0,0 +1,13 @@
+const docUrl = 'https://docs.openpitrix.io';


docs link is a global conf, can be placed at src/config/index.js, or server/local_config.yml

No need to separate version, lang

sunnywx · 2018-12-05T06:45:03Z

src/pages/Dashboard/Apps/Add/Card/index.jsx

+@observer
+export default class CreateAppCard extends Component {
+  static propTypes = {
+    appCreateStore: PropTypes.object,


I think this appCreateStore is no need. External store will be passed down to this component.

sunnywx · 2018-12-05T06:45:41Z

src/pages/Dashboard/Apps/Add/Card/index.jsx

+export default class CreateAppCard extends Component {
+  static propTypes = {
+    appCreateStore: PropTypes.object,
+    children: PropTypes.node,


No need to add children prop, since it's implicit

sunnywx · 2018-12-05T06:46:53Z

src/pages/Dashboard/Apps/Add/Card/index.jsx

+            size={20}
+          />
+        )}
+        <Icon name={icon} size={48} className={styles.icon} type={'light'} />


refactor it to type='light'

sunnywx · 2018-12-05T06:47:49Z

src/pages/Dashboard/Apps/Card/index.jsx

+import styles from './index.scss';
+
+@withRouter
+export default class Loading extends Component {


Why this component name is Loading ?

I forget rename it ...

sunnywx · 2018-12-05T06:48:26Z

src/pages/Dashboard/Apps/Card/index.jsx

+import { withRouter } from 'react-router';
+import _ from 'lodash';
+
+import { getPastTime } from 'src/utils';


Place utils after user-defined components

sunnywx · 2018-12-05T08:30:24Z

src/components/Loading/index.js

@@ -0,0 +1,3 @@
+import Loading from './index.jsx';


This file can be removed

sunnywx · 2018-12-05T08:31:38Z

src/components/Loading/index.jsx

@@ -16,12 +17,18 @@ export default class Loading extends Component {
  };

  render() {
-    const { className, isLoading, children } = this.props;
+    const {
+      className, grayColor, isLoading, children


I think the name grayColor is not good. What does that mean?

liiil825 added the v0.4 label Nov 26, 2018

liiil825 requested review from sunnywx and whDongRui November 26, 2018 10:15

sunnywx removed the v0.4 label Nov 26, 2018

liiil825 changed the title ~~[WIP] Add app and list~~ [WIP] Add app page and list page Nov 27, 2018

liiil825 changed the title ~~[WIP] Add app page and list page~~ [WIP] Add app page and list of apps page Nov 27, 2018

liiil825 force-pushed the feature/v0.4-app branch from ecf2de8 to 4751aba Compare November 27, 2018 06:33

sunnywx changed the title ~~[WIP] Add app page and list of apps page~~ [WIP] App mgmt pages Nov 27, 2018

liiil825 added 3 commits November 29, 2018 14:25

feat: Create app

2f59a50

feat: Add upload config style for create app

783ee7d

refactor: Remove Dashboard/Header component

1ef96a1

liiil825 force-pushed the feature/v0.4-app branch from 4751aba to 1ef96a1 Compare November 29, 2018 06:58

liiil825 added 2 commits November 29, 2018 15:53

fix: External url for Link

a26f3e4

fix: Some eslint warning

3ea6097

liiil825 changed the title ~~[WIP] App mgmt pages~~ App mgmt pages Nov 29, 2018

fix: Disable SVG file upload

7d11ed0

whDongRui reviewed Dec 3, 2018

View reviewed changes

src/components/Base/index.js Outdated Show resolved Hide resolved

whDongRui reviewed Dec 3, 2018

View reviewed changes

src/components/Base/Link/index.jsx Outdated Show resolved Hide resolved

whDongRui reviewed Dec 3, 2018

View reviewed changes

src/components/Base/Link/index.jsx Outdated Show resolved Hide resolved

liiil825 added 4 commits December 3, 2018 11:29

feat: Add app version

16ddf65

fix: Rename Link to BaseLink

7edf71d

feat: Add upload validate package

332806a

test: Add Base/Link test

7a2ff1d

sunnywx reviewed Dec 5, 2018

View reviewed changes

liiil825 added 3 commits December 5, 2018 15:26

fix: Code review

22b32d0

fix: Add a config for doc url

6bf33fe

refactor: Cehckout component of Base/Input

0f825f8

sunnywx reviewed Dec 5, 2018

View reviewed changes

liiil825 added 3 commits December 5, 2018 17:24

fix: Code review

90b146a

fix: Checkout Base/Input

5bcf9cd

fix: Change valueChange to onChange

720b331

sunnywx approved these changes Dec 5, 2018

View reviewed changes

sunnywx merged commit 3c14060 into openpitrix:master Dec 5, 2018

		@@ -0,0 +1,37 @@
		import React, { Component, Fragment } from 'react';

		@@ -0,0 +1,13 @@
		const docUrl = 'https://docs.openpitrix.io';

App mgmt pages #552

App mgmt pages #552

Conversation

liiil825 commented Nov 26, 2018

codecov bot commented Nov 26, 2018 • edited Loading

Codecov Report

liiil825 commented Dec 5, 2018

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

liiil825 Dec 5, 2018 • 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

Choose a reason for hiding this comment

sunnywx Dec 5, 2018 • 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

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

Choose a reason for hiding this comment

codecov bot commented Nov 26, 2018 •

edited

Loading

liiil825 Dec 5, 2018 •

edited

Loading

sunnywx Dec 5, 2018 •

edited

Loading