use office-fabric-ui components #1964

lvybriage · 2020-01-16T07:43:21Z

This PR‘s content:

use office-fabric-ui , delete antd dependency
refactor nav bar
refactor addColumn modal

improvement

table：击selecte icon可以选中一行但不需要打开expand row [调试中，有bug, 期待bug bash进]
trial's intermediate result graph: x轴名称改为trial sequence id

support items in next release:

Parameters | Log in open row: div保持一样的高度
tableList pagination function
单独的refresh button function

QuanluZhang · 2020-02-03T06:59:25Z

src/webui/src/components/trial-detail/TableList.tsx

@@ -373,136 +388,132 @@ class TableList extends React.Component<TableListProps, TableListState> {
                parameterStr.push(`${value} (search space)`);
            });
        }
+        let showTitle = COLUMNPro; // eslint-disable-line @typescript-eslint/no-unused-vars
        showTitle = COLUMNPro.concat(parameterStr);

        // only succeed trials have final keys
        if (tableSource.filter(record => record.status === 'SUCCEEDED').length >= 1) {
            const temp = tableSource.filter(record => record.status === 'SUCCEEDED')[0].accuracy;


temp: better to give it a meaningful name.

QuanluZhang · 2020-02-03T07:01:18Z

src/webui/src/static/const.ts

@@ -2,8 +2,9 @@
 const METRIC_GROUP_UPDATE_THRESHOLD = 100;
 const METRIC_GROUP_UPDATE_SIZE = 20;

-const MANAGER_IP = `/api/v1/nni`;
+const MANAGER_IP = `http://13.77.78.63:8080/api/v1/nni`;


remove this ip address?

SparkSnail · 2020-02-03T07:45:25Z

src/webui/config/webpackDevServer.config.js

    watchOptions: {
      ignored: ignoredFiles(paths.appSrc),
    },
    // Enable HTTPS if the HTTPS environment variable is set to 'true'
    https: protocol === 'https',
-    host: host,
+    host,


why remove host:

同名可以简写~ 这是项目生成时的文件，我其实并没有改什么...

SparkSnail · 2020-02-03T07:47:15Z

src/webui/scripts/build.js

+        'Setting NODE_PATH to resolve modules absolutely has been deprecated in favor of setting baseUrl in jsconfig.json (or tsconfig.json if you are using TypeScript) and will be removed in a future major release of create-react-app.'
+      )
+    );
+    console.log();


empty content

I had delete this line.

SparkSnail · 2020-02-03T07:48:01Z

src/webui/scripts/start.js

-  console.log(`Learn more here: ${chalk.yellow('http://bit.ly/2mwWSwH')}`);
+  console.log(
+    `Learn more here: ${chalk.yellow('https://bit.ly/CRA-advanced-config')}`
+  );
  console.log();


empty content

Thanks for reviewing! I had delete this line.

SparkSnail · 2020-02-03T07:51:26Z

src/webui/src/components/trial-detail/Intermediate.tsx

-            if (isFilter !== this.state.isFilter) {
-                return true;
-            }
+    //         if (isFilter !== this.state.isFilter) {


why annotate these lines?

// will add it later. [graph is not render when add this component]
So... will delete this notes. will support this function after that.

SparkSnail · 2020-02-03T07:53:16Z

src/webui/src/components/trial-detail/Para.tsx

-            if (whichGraph !== beforeGraph) {
-                return true;
-            }
+    //     const { whichGraph } = nextProps;


If these lines are not useful, could remove them.

SparkSnail · 2020-02-03T08:01:12Z

src/webui/src/static/function.ts

                // render the table
                if (updateList) {
                    updateList();  // FIXME
                }
            } else {
-                message.error('fail to cancel the job');
+                alert('fail to cancel the job');
+                // message.error('fail to cancel the job');


remove unused function?

this function will use in [kill a trial],
I 'll update this function in next commit in PR.
Thanks for reviewing.

liuzhe-lz · 2020-02-07T07:01:04Z

src/webui/LICENSE

@@ -0,0 +1,21 @@
+    MIT License


Is this file necessary?

I think so...

liuzhe-lz · 2020-02-07T07:03:13Z

src/webui/src/App.test.tsx

+import App from './App';
+
+it('renders without crashing', () => {
+  const div = document.createElement('div');


Please keep indentation consistent.

liuzhe-lz · 2020-02-07T07:21:41Z

src/webui/src/static/function.ts

                // render the table
                if (updateList) {
                    updateList();  // FIXME
                }
            } else {
-                message.error('fail to cancel the job');
+                alert('fail to cancel the job');


I believe using browser alert API is a bad practice.
We should port to a better notification framework later.

你说的对！下一版会fix

liuzhe-lz · 2020-02-07T07:38:15Z

src/webui/src/components/overview/SuccessTable.tsx

+        });
+    };
+
+    private _copyAndSort<T>(items: T[], columnKey: string, isSortedDescending?: boolean): T[] {


Does TS/React recommend variable name starting with underscore?

fabric-react's demos show that,
but,
将会改成驼峰命名！

liuzhe-lz · 2020-02-07T07:38:42Z

src/webui/src/components/overview/Progress.tsx

@@ -19,24 +19,105 @@ interface ProgressProps {

 interface ProgressState {
    isShowLogDrawer: boolean;
+    isCalloutVisible?: boolean;


when experiment status is error, hover to the status, will tooltip a modal.
isCalloutVisible decide the modal's hidden | show

liuzhe-lz · 2020-02-07T07:40:04Z

src/webui/src/components/overview/Progress.tsx

    editTrialConcurrency = async (userInput: string): Promise<void> => {
        if (!userInput.match(/^[1-9]\d*$/)) {
-            message.error('Please enter a positive integer!', 2);
+            this.getMessageInfo('Please enter a positive integer!', 'error');


I think "get message info" is misleading.

yeah, will change to showMessageInfo
It's OK?

Ubuntu added 2 commits January 16, 2020 03:52

Use office fabric ui components

fa030c1

fix own comments and eslint

4b6cd5b

lvybriage requested review from liuzhe-lz and chicm-ms January 16, 2020 07:43

deshui added 4 commits January 17, 2020 14:34

show message info when click buttons

7910e95

fix overview style

7a132eb

fix some detail page style

f76322f

fix failed PR

865a5c2

lvybriage removed request for chicm-ms and liuzhe-lz January 20, 2020 03:30

Lijiao added 2 commits January 20, 2020 14:14

fix failed PR

2b4e139

test

dd15322

lvybriage mentioned this pull request Jan 20, 2020

Test #1974

Closed

Lijiao and others added 3 commits January 21, 2020 03:53

rollback yarn.lock to fix failed UI

e6ebeea

fix detail style

8920cc3

update nav and detail modal style

1749c2b

lvybriage requested review from liuzhe-lz, chicm-ms, SparkSnail and ultmaster February 3, 2020 02:42

refresh selectors

d868fab

QuanluZhang reviewed Feb 3, 2020

View reviewed changes

SparkSnail reviewed Feb 3, 2020

View reviewed changes

liuzhe-lz reviewed Feb 7, 2020

View reviewed changes

seepoint added 2 commits February 9, 2020 23:28

fix comments of team

c189a1b

fix comments of Zhe

783c7bb

SparkSnail approved these changes Feb 10, 2020

View reviewed changes

lvybriage mentioned this pull request Feb 10, 2020

Values on the x axis lack of units #2021

Closed

chicm-ms approved these changes Feb 10, 2020

View reviewed changes

chicm-ms merged commit c718794 into microsoft:master Feb 10, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

use office-fabric-ui components #1964

use office-fabric-ui components #1964

lvybriage commented Jan 16, 2020 •

edited

Loading

QuanluZhang Feb 3, 2020

lvybriage Feb 5, 2020

QuanluZhang Feb 3, 2020

lvybriage Feb 5, 2020

SparkSnail Feb 3, 2020

lvybriage Feb 5, 2020

SparkSnail Feb 3, 2020

lvybriage Feb 5, 2020

SparkSnail Feb 3, 2020

lvybriage Feb 5, 2020

SparkSnail Feb 3, 2020

lvybriage Feb 5, 2020

SparkSnail Feb 3, 2020

lvybriage Feb 5, 2020

SparkSnail Feb 3, 2020

lvybriage Feb 5, 2020

liuzhe-lz Feb 7, 2020

lvybriage Feb 9, 2020

liuzhe-lz Feb 7, 2020

lvybriage Feb 9, 2020

liuzhe-lz Feb 7, 2020

lvybriage Feb 9, 2020

liuzhe-lz Feb 7, 2020

lvybriage Feb 9, 2020

liuzhe-lz Feb 7, 2020

lvybriage Feb 9, 2020

liuzhe-lz Feb 7, 2020

lvybriage Feb 9, 2020

use office-fabric-ui components #1964

use office-fabric-ui components #1964

Conversation

lvybriage commented Jan 16, 2020 • 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

lvybriage commented Jan 16, 2020 •

edited

Loading