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.

Sign up for GitHub

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

Jump to bottom

feat: Avatar.Group #25192

Merged
merged 27 commits into from
Jul 13, 2020
Merged

feat: Avatar.Group #25192

merged 27 commits into from
Jul 13, 2020

Conversation

xrkffgg
Copy link
Member

@xrkffgg xrkffgg commented Jun 24, 2020 •
edited by afc163
Loading

[中文版模板 / Chinese template]

🤔 This is a ...

  • New feature
  • Bug fix
  • Site / documentation update
  • Demo update
  • Component style update
  • TypeScript definition update
  • Bundle size optimization
  • Perfermance optimization
  • Refactoring
  • Code style optimization
  • Test Case
  • Branch merge
  • Other (about what?)

🔗 Related issue link

💡 Background and solution

📝 Changelog

Language Changelog
🇺🇸 English Add Avatar.Group for showing a set of avatars.
🇨🇳 Chinese 新增 Avatar.Group 用于展现一组头像集合。

☑️ Self Check before Merge

⚠️ Please check all items below before review. ⚠️

  • Doc is updated/provided or not needed
  • Demo is updated/provided or not needed
  • TypeScript definition is updated/provided or not needed
  • Changelog is provided or not needed

View rendered components/avatar/demo/group.md

@ant-design-bot
Copy link
Contributor

ant-design-bot commented Jun 24, 2020 •
edited
Loading

@ant-design-bot
Copy link
Contributor

ant-design-bot commented Jun 24, 2020 •
edited
Loading

@codesandbox-ci
Copy link

codesandbox-ci bot commented Jun 24, 2020 •
edited
Loading

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit e5d3fc6:

Sandbox Source
antd reproduction template Configuration

@fireairforce

This comment has been minimized.

Sign in to view
@xrkffgg

This comment has been minimized.

Sign in to view
@afc163
Copy link
Member

afc163 commented Jun 24, 2020

可以参考一下 https://v2-pro.ant.design/components/avatar-list-cn

@xrkffgg xrkffgg changed the title feat: Avatar.Group [WIP] feat: Avatar.Group Jun 24, 2020
@xrkffgg

This comment has been minimized.

Sign in to view
@xrkffgg

This comment has been minimized.

Sign in to view
@afc163
Copy link
Member

afc163 commented Jun 24, 2020

hover 上去给 Popover 吧。

@xrkffgg xrkffgg changed the title [WIP] feat: Avatar.Group feat: Avatar.Group Jun 25, 2020
@xrkffgg

This comment has been minimized.

Sign in to view
@xrkffgg xrkffgg requested review from zombieJ, afc163 and yoyo837 June 25, 2020 12:44
@xrkffgg

This comment has been minimized.

Sign in to view
@zombieJ
Copy link
Member

zombieJ commented Jun 26, 2020

感觉不需要 hovered。hover 到 Avatar 上显示的 tooltip 展示用户名。最后 +2 那个 popover 感觉要斟酌一下，如果用户很多，这弹出来还是一坨。

@xrkffgg

This comment has been minimized.

Sign in to view
@zombieJ

This comment has been minimized.

Sign in to view
@xrkffgg

This comment has been minimized.

Sign in to view
@xrkffgg xrkffgg force-pushed the feat-avatargroup branch from d5d4522 to fce8456 Compare June 29, 2020 01:48
xrkffgg
xrkffgg commented Jun 29, 2020
View reviewed changes
components/avatar/avatar.tsx Outdated
onError?: () => boolean;
}

const InternalAvatar: React.ForwardRefRenderFunction<unknown, AvatarProps> = props => {

This comment was marked as outdated.

Sign in to view

This comment was marked as outdated.

Sign in to view

components/avatar/index.tsx
/* return false to prevent Avatar show default fallback behavior, then you can do fallback by your self */
onError?: () => boolean;
interface CompoundedComponent
extends React.ForwardRefExoticComponent<AvatarProps & React.RefAttributes<HTMLElement>> {

This comment was marked as outdated.

Sign in to view

@xrkffgg xrkffgg changed the title feat: Avatar.Group [WIP] feat: Avatar.Group Jun 29, 2020
@xrkffgg xrkffgg force-pushed the feat-avatargroup branch from 4b180c5 to 48f6a17 Compare June 29, 2020 08:09
@chenshuai2144
Copy link
Contributor

微软爸爸也有样式可以看一下

https://developer.microsoft.com/en-us/fabric-js/components/peoplepicker/peoplepicker

@xrkffgg
Copy link
Member Author

xrkffgg commented Jul 12, 2020

  • useContext
  • toArray
  • API naming

Fixed. PTAL

@xrkffgg xrkffgg requested a review from zombieJ July 12, 2020 11:29
afc163
afc163 reviewed Jul 12, 2020
View reviewed changes
components/avatar/index.zh-CN.md Outdated
| ------------------------- | -------------------- | ----------------- | ------ | ---- |
| maxAvatarCount | 显示的最大头像个数 | number | - | |
| maxAvatarStyle | 多余头像样式 | CSSProperties | - | |
| maxAvatarPopoverPlacement | 多余头像气泡弹出位置 | `top` \| `bottom` | `top` | |
Copy link
Member

Choose a reason for hiding this comment

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

maxCount 应该就行了，Avatar 有点多余。

Copy link
Member Author

Choose a reason for hiding this comment

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

ok

zombieJ
zombieJ reviewed Jul 12, 2020
View reviewed changes
components/avatar/group.tsx Outdated
className,
);

const renderChildren = () => {
Copy link
Member

Choose a reason for hiding this comment

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

看起来这个 renderChildren 没啥意义，直接打平就行了。

hengkx
hengkx reviewed Jul 13, 2020
View reviewed changes
components/avatar/index.zh-CN.md
| 参数 | 说明 | 类型 | 默认值 | 版本 |
| ------------------- | -------------------- | ----------------- | ------ | ---- |
| maxCount | 显示的最大头像个数 | number | - | |
| maxStyle | 多余头像样式 | CSSProperties | - | |
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
Member Author

Choose a reason for hiding this comment

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

😂😂 我太难了，一人一个说法

afc163
afc163 reviewed Jul 13, 2020
View reviewed changes
components/avatar/index.en-US.md
| 参数 | 说明 | 类型 | 默认值 | 版本 |
| ------------------- | -------------------------------------- | ----------------- | ------ | ---- |
| maxCount | Max avatars to show | number | - | |
| maxStyle | The style of excess avatar style | CSSProperties | - | |
Copy link
Member

Choose a reason for hiding this comment

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

maxStyle 看上去没啥用，可以先删掉。

Copy link
Member Author

Choose a reason for hiding this comment

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

image

去掉后，默认的 +2 显示成这样了

Copy link
Member

Choose a reason for hiding this comment

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

明白了，那留着吧。

@xrkffgg
Copy link
Member Author

xrkffgg commented Jul 13, 2020

/rebase

@xrkffgg
Copy link
Member Author

xrkffgg commented Jul 13, 2020

这个 CI 好像没关系

@xrkffgg xrkffgg merged commit 877caa2 into feature Jul 13, 2020
@xrkffgg xrkffgg deleted the feat-avatargroup branch July 13, 2020 04:45
@07akioni 07akioni mentioned this pull request Jul 26, 2020
Merged
@hsuanxyz hsuanxyz mentioned this pull request Oct 10, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hengkx hengkx hengkx left review comments

@zombieJ zombieJ zombieJ left review comments

@afc163 afc163 afc163 approved these changes

@yoyo837 yoyo837 Awaiting requested review from yoyo837

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@xrkffgg @ant-design-bot @fireairforce @afc163 @zombieJ @chenshuai2144 @hengkx