Skip to content
This repository has been archived by the owner on Sep 18, 2024. It is now read-only.

Chevron icon before table row and TableList refactoring #2900

Merged
merged 24 commits into from
Oct 14, 2020

Conversation

ultmaster
Copy link
Contributor

@ultmaster ultmaster commented Sep 16, 2020

Will do in this PR.

  • Integrate ChangeColumnComponent with Para.
  • Status text color.
  • Metric text LATEST/FINAL tag.

Should fix (high prior):

  • Table column widths are weird (need help).

Should test:

  • On table items change (auto-update).

Known issues (low pri):

  • Default metric is treated ad-hoc-ly and supports intermediates (displayed as LATEST), while others don't.
  • Active page number is not correct when number of items per page has changed.
  • s[0] undefined in Compare when closing sometimes.

@QuanluZhang QuanluZhang mentioned this pull request Sep 23, 2020
79 tasks
@ultmaster ultmaster marked this pull request as ready for review October 9, 2020 03:38
@Lijiaoa
Copy link
Contributor

Lijiaoa commented Oct 9, 2020

these items had fixed by ultmaster:

image

  • Add/Remove columns, Search by status also Search by parameters

image

  • The column (Start time and End time) is hidden when init
  • Could you fix this lint?
    image

minWidth: columnWidth * 4,
maxWidth: columnWidth * 10,
isResizable: true,
onColumnClick: this._onColumnClick.bind(this),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when page auto refresh, these trials sorted status will be reset. so you could use this function in render(): example

Comment on lines 55 to 59
if (a[key] === undefined) {
return 1;
}
if (b[key] === undefined) {
return -1;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because default metric column may be NaN | Infinity or others, so you should change it back. example

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated and refactored this part. Please check.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check the number column sort: duration and default metric.

@QuanluZhang QuanluZhang requested a review from liuzhe-lz October 9, 2020 08:52
@Lijiaoa
Copy link
Contributor

Lijiaoa commented Oct 10, 2020

  • 排序number
  • trial expand row 被刷新闭合掉
  • customized trial后续提示成功失败的model窗口不出现
  • 只有running trial时不显示default metric列

class ExpandableDetails extends React.Component<ExpandableDetailsProps, ExpandableDetailsState> {
constructor(props: ExpandableDetailsProps) {
super(props);
this.state = { isExpand: false };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems like that you don't use this.state.isExpand

Comment on lines 288 to 289
minWidth: columnWidth * 4,
maxWidth: columnWidth * 10,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can try this size:
minWidth: columnWidth * 13,
maxWidth: columnWidth * 18,

@QuanluZhang QuanluZhang changed the base branch from master to v1.9 October 12, 2020 01:14
@QuanluZhang QuanluZhang merged commit 88a225f into microsoft:v1.9 Oct 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants