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

feat: add Breadcrumb.Separator #17873

Merged
merged 4 commits into from Aug 2, 2019
Merged

feat: add Breadcrumb.Separator #17873

merged 4 commits into from Aug 2, 2019

Conversation

quicksand-1
Copy link
Contributor

@quicksand-1 quicksand-1 commented Jul 25, 2019

🤔 这个变动的性质是?

  • 新特性提交
  • 日常 bug 修复
  • 站点、文档改进
  • 组件样式改进
  • TypeScript 定义更新
  • 重构
  • 代码风格优化
  • 测试用例
  • 分支合并
  • 其他改动(是关于什么的改动?)

🔗 相关 Issue

close #17812

💡 需求背景和解决方案

需求背景

  • 在使用 Breadcrumb.Item 需要自定义 separator 时,Item 组件会被父组件覆盖。如需要渲染一下样式:
    • 当前位置:xxx / xxx / xxx

解决方案

  • 新增 Breadcrumb.Separator 组件进行 separator 的自定义。

📝 更新日志怎么写?

语言 更新描述
🇺🇸 英文
🇨🇳 中文 新增了 Breadcrumb.Separator 组件,可进行 separator 自定义

☑️ 请求合并前的自查清单

  • 文档已补充或无须补充
  • 代码演示已提供或无须提供
  • TypeScript 定义已补充或无须补充
  • Changelog 已提供或无须提供

View rendered components/breadcrumb/demo/separator-1.md

@netlify
Copy link

netlify bot commented Jul 25, 2019

Deploy preview for ant-design ready!

Built with commit b91dcd5

https://deploy-preview-17873--ant-design.netlify.com

<Breadcrumb.Item>Home</Breadcrumb.Item>
<Breadcrumb>
<Breadcrumb.Item separator=":">Location</Breadcrumb.Item>
<Breadcrumb.Item separator="---">Home</Breadcrumb.Item>
Copy link
Member

Choose a reason for hiding this comment

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

不要支持这种用法。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Item 不支持 separator 的考虑是什么呢?
我发现新增一个 Breadcrumb.Separator 改动量很大。如果仅仅需要让 Item 支持自定义的 separator 属性,只需要改如下图所示的代码即可。
image

Copy link
Member

@afc163 afc163 Jul 30, 2019

Choose a reason for hiding this comment

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

最好只推荐一种使用方式,否则 <Breadcrumb.Item separator=":" /><Breadcrumb.Separator>:</Breadcrumb.Separator> 两个写法完全等效,共存没什么意义。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

是的,在最开始的时候,我在设计 Breadcrumb.Separator 组件的时候,发现也需要改其他文件的代码,且改动量不小。所以最终决定使用 <Breadcrumb.Item separator=":" /> 。

Copy link
Member

Choose a reason for hiding this comment

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

从语义上来说 Breadcrumb.Separator 好很多。

@codecov
Copy link

codecov bot commented Jul 31, 2019

Codecov Report

Merging #17873 into feature will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           feature   #17873      +/-   ##
===========================================
+ Coverage    96.12%   96.12%   +<.01%     
===========================================
  Files          271      272       +1     
  Lines         7478     7489      +11     
  Branches      2052     2056       +4     
===========================================
+ Hits          7188     7199      +11     
  Misses         288      288              
  Partials         2        2
Impacted Files Coverage Δ
components/breadcrumb/index.tsx 100% <100%> (ø) ⬆️
components/breadcrumb/Breadcrumb.tsx 96.66% <100%> (+0.05%) ⬆️
components/breadcrumb/BreadcrumbSeparator.tsx 100% <100%> (ø)
components/breadcrumb/BreadcrumbItem.tsx 100% <100%> (ø) ⬆️

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 8479a7d...06e2e1a. Read the comment docs.

@codecov
Copy link

codecov bot commented Jul 31, 2019

Codecov Report

Merging #17873 into feature will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           feature   #17873      +/-   ##
===========================================
+ Coverage    96.12%   96.12%   +<.01%     
===========================================
  Files          271      272       +1     
  Lines         7478     7489      +11     
  Branches      2052     2099      +47     
===========================================
+ Hits          7188     7199      +11     
  Misses         288      288              
  Partials         2        2
Impacted Files Coverage Δ
components/breadcrumb/index.tsx 100% <100%> (ø) ⬆️
components/breadcrumb/Breadcrumb.tsx 96.66% <100%> (+0.05%) ⬆️
components/breadcrumb/BreadcrumbSeparator.tsx 100% <100%> (ø)
components/breadcrumb/BreadcrumbItem.tsx 100% <100%> (ø) ⬆️

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 8479a7d...b91dcd5. Read the comment docs.

</Breadcrumb>,
mountNode,
);
```
Copy link
Member

Choose a reason for hiding this comment

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

separator-1.md => separator-indepent.md

| ------- | -------------- | -------------------------------------- | ------ | ------ |
| href | 链接的目的地 | string | - | 3.17.0 |
| overlay | 下拉菜单的内容 | [Menu](/components/menu) \| () => Menu | - | 3.17.0 |
| onClick | 单击事件 | (e:MouseEvent)=>void | - | 3.17.0 |

### routes
Copy link
Member

Choose a reason for hiding this comment

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

文档里要加一下 Breadcrumb.Separator 的说明。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

说明的表格中的“版本”字段该如何填写?

Copy link
Member

Choose a reason for hiding this comment

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

3.21.0 好了,赶上车这个月底就发了。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image

Copy link
Member

Choose a reason for hiding this comment

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

👍

@@ -32,6 +32,14 @@ title: Breadcrumb
| overlay | 下拉菜单的内容 | [Menu](/components/menu) \| () => Menu | - | 3.17.0 |
| onClick | 单击事件 | (e:MouseEvent)=>void | - | 3.17.0 |

### Breadcrumb.Separator
Copy link
Member

Choose a reason for hiding this comment

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

可以在这后面标一个 3.21.0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

语义化提交或者 PR 标题有规范吗?这个 PR 提交还需要修改吗❓

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
### Breadcrumb.Separator
### Breadcrumb.Separator `3.21.0`

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image

@afc163 afc163 changed the title add Breadcrumb.Separator feat: add Breadcrumb.Separator Aug 1, 2019
@afc163 afc163 merged commit 36db887 into ant-design:feature Aug 2, 2019
@ikobe-zz ikobe-zz mentioned this pull request Aug 4, 2019
14 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

2 participants