Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New Component PageHeader #13637

Merged
merged 61 commits into from Mar 2, 2019
Merged

New Component PageHeader #13637

merged 61 commits into from Mar 2, 2019

Conversation

chenshuai2144
Copy link
Contributor

@chenshuai2144 chenshuai2144 commented Dec 15, 2018

New Component

Add new components to support more usage scenarios

Preview

image

Impact on users and possible risks

Add new components without affecting existing code.

changelog

add new components PageHeader

request a self-check list before merging

  • document has been added
  • Code demo is provided
  • TypeScript definition has been added
  • Changelog not required

@chenshuai2144
Copy link
Contributor Author

image

@ant-design ant-design deleted a comment from codecov bot Dec 15, 2018
@ant-design ant-design deleted a comment from codecov bot Dec 15, 2018
@codecov
Copy link

codecov bot commented Dec 15, 2018

Codecov Report

Merging #13637 into feature will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           feature   #13637   +/-   ##
========================================
  Coverage    93.97%   93.97%           
========================================
  Files          314      314           
  Lines         6939     6939           
  Branches      1759     1759           
========================================
  Hits          6521     6521           
  Misses         404      404           
  Partials        14       14

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 812c28f...a2fae3f. Read the comment docs.

@chenshuai2144 chenshuai2144 changed the title new components PageHeader [WIP] new components PageHeader Dec 15, 2018
@ant-design ant-design deleted a comment from netlify bot Dec 15, 2018
@ant-design ant-design deleted a comment from codecov bot Dec 15, 2018
@codecov
Copy link

codecov bot commented Dec 16, 2018

Codecov Report

Merging #13637 into feature will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           feature   #13637   +/-   ##
========================================
  Coverage    93.97%   93.97%           
========================================
  Files          314      314           
  Lines         6939     6939           
  Branches      1759     1759           
========================================
  Hits          6521     6521           
  Misses         404      404           
  Partials        14       14

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 812c28f...0ac7232. Read the comment docs.

@codecov
Copy link

codecov bot commented Dec 16, 2018

Codecov Report

Merging #13637 into feature will increase coverage by 0.08%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           feature   #13637      +/-   ##
===========================================
+ Coverage    93.08%   93.16%   +0.08%     
===========================================
  Files          247      248       +1     
  Lines         6635     6670      +35     
  Branches      1940     1928      -12     
===========================================
+ Hits          6176     6214      +38     
+ Misses         456      455       -1     
+ Partials         3        1       -2
Impacted Files Coverage Δ
components/index.tsx 100% <ø> (ø) ⬆️
components/page-header/index.tsx 100% <100%> (ø)
components/time-picker/index.tsx 81.94% <0%> (ø) ⬆️
components/transfer/index.tsx 85.43% <0%> (ø) ⬆️
components/_util/wave.tsx 87.5% <0%> (+2.88%) ⬆️

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 5ce7e71...a0e7077. Read the comment docs.

@zombieJ
Copy link
Member

zombieJ commented Dec 17, 2018

  • add subTitle
  • remove logo, keep extraContent
  • content -> children
  • action 和 Card 一样叫 extra
  • Breadcrumb or Tabsbreadcrumb={<Breadcrumb>{...}</Breadcrumb>}? 感觉也不太好

或者 下方的操作和标签移到 renderFooter 里让用户自己搞算了。否则下面这个看着有点蛋疼。

@chenshuai2144 @afc163

@yutingzhao1991
Copy link
Contributor

问一个问题,这个不做在 ant-design-pro 里面?我们按照什么标准来划定一个组件是在 antd 还是再 ant-design-pro 里面呢?

@chenshuai2144
Copy link
Contributor Author

扭头问一下你左边的男人

<style>
.ant-pageheader{
border: 1px solid rgb(235, 237, 240);
}
Copy link
Member

@afc163 afc163 Dec 17, 2018

Choose a reason for hiding this comment

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

indent 和 space 不对。

| action | Operating area, at the end of the line of the title line | ReactNode | - |
| logo | logo | ReactNode | - |
| content | content | ReactNode | - |
| extraContent | Extra content area, on the right side of content | ReactNode | - |
Copy link
Member

Choose a reason for hiding this comment

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

extraContent 和 action 的命名还是太不直观了。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

action 感觉还不错,extraContent的确感觉有点扯。
不如叫 rightContent 和 leftContent 吧

@afc163
Copy link
Member

afc163 commented Dec 18, 2018

extraContent 去掉好了,感觉用的很少。

@chenshuai2144
Copy link
Contributor Author

extraContent 还是很有用的。这两种如果有extraContent 实现起来很容易
image

@zombieJ
Copy link
Member

zombieJ commented Dec 18, 2018

Card 里右上角叫 extra,这边也这么叫吧。叫 action 又要多记一个。

@zombieJ
Copy link
Member

zombieJ commented Dec 18, 2018

extraContent 或者让用户自己用 Flex 去实现?

=============

update:
直接 content 和 extraContent 合成一个 children,让用户自己来用。3 和 4 的例子,感觉还是不够抽象。

@@ -14,13 +14,16 @@ title:
Basic Page Header

```jsx
import { PageHeader } from 'antd';
import { PageHeader } from "antd";
Copy link
Member

Choose a reason for hiding this comment

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

应该是单引号。

@yutingzhao1991 yutingzhao1991 mentioned this pull request Dec 26, 2018
13 tasks
if (breadcrumb && breadcrumb.routes && breadcrumb.routes.length > 2) {
return this.renderBreadcrumb();
}
return this.renderBack(prefixCls);
Copy link
Member

Choose a reason for hiding this comment

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

renderBack 可以不出现么?感觉很多场景不需要返回。

@@ -7,11 +7,11 @@ title:

## zh-CN

标准页头
标准页头,适合使用在需要简单描述的场景
Copy link
Member

Choose a reason for hiding this comment

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

标点符号

expect(wrapper.find('.ant-page-header-back-icon')).toHaveLength(0);
});

it('pageHeader should contain back it back', async () => {
Copy link
Member

Choose a reason for hiding this comment

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

目测用例都不需要 async。

@Akatsukikikiki
Copy link

  • 有几处间距,以及 icon 的尺寸需要调整下:

2019-01-15 3 05 30

2019-01-15 3 12 59

  • 简介少了句号:

2019-01-15 3 15 34

@zombieJ
Copy link
Member

zombieJ commented Jan 15, 2019

这个晚点合,等 Paragraph 好了用它代替 p。

@Akatsukikikiki
Copy link

padding 没有把 button 包进去:

2019-01-15 3 28 42

2019-01-15 3 28 49

@zombieJ zombieJ mentioned this pull request Feb 19, 2019
13 tasks
zombieJ
zombieJ previously approved these changes Feb 19, 2019
padding-bottom: 8px;
white-space: nowrap;
line-height: 20px;
}
Copy link
Member

Choose a reason for hiding this comment

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

这些 css 多余了吧。

Copy link
Member

Choose a reason for hiding this comment

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

Description 直接用新的 Descriptions 吧,这里代码好冗余。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

我想的是先把他弄出去,众测一波,不知道api 设计的合不合理

@afc163
Copy link
Member

afc163 commented Feb 19, 2019

预览链接在哪里。

@chenshuai2144
Copy link
Contributor Author

@zombieJ
Copy link
Member

zombieJ commented Feb 28, 2019

CI 修一下,准备合了

@chenshuai2144 chenshuai2144 merged commit dee8f93 into feature Mar 2, 2019
@chenshuai2144 chenshuai2144 deleted the page-header branch March 2, 2019 07:51
@afc163 afc163 mentioned this pull request Mar 30, 2019
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet