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(Dropdown): add Component Dropdown #1106

Closed
wants to merge 15 commits into from

Conversation

JunIce
Copy link
Contributor

@JunIce JunIce commented May 26, 2023

@vercel
Copy link

vercel bot commented May 26, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
zarm ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 12, 2023 3:08am

@github-actions
Copy link

github-actions bot commented May 26, 2023

🎊 PR Preview has been successfully built and deployed to https://zarm-preview-pr-1106.surge.sh

@vercel vercel bot temporarily deployed to Preview – zarm May 26, 2023 09:32 Inactive
@codecov
Copy link

codecov bot commented May 26, 2023

Codecov Report

Merging #1106 (1ba949a) into master (2e6df07) will decrease coverage by 0.15%.
The diff coverage is 87.50%.

@@            Coverage Diff             @@
##           master    #1106      +/-   ##
==========================================
- Coverage   90.33%   90.19%   -0.15%     
==========================================
  Files         237      240       +3     
  Lines        5245     5373     +128     
  Branches     1194     1265      +71     
==========================================
+ Hits         4738     4846     +108     
- Misses        497      517      +20     
  Partials       10       10              
Impacted Files Coverage Δ
packages/zarm/src/use-scroll/index.ts 100.00% <ø> (+7.69%) ⬆️
packages/zarm/src/dropdown/DropdownItem.tsx 85.71% <85.71%> (ø)
packages/zarm/src/dropdown/Dropdown.tsx 87.15% <87.15%> (ø)
packages/zarm/src/dropdown/index.tsx 100.00% <100.00%> (ø)
packages/zarm/src/index.ts 100.00% <100.00%> (ø)

... and 7 files with indirect coverage changes

@JeromeLin
Copy link
Collaborator

JeromeLin commented May 26, 2023

@JunIce
1.展开的时候需要下拉动画,看看能不能用 Transition 组件 传入自定义动画来实现
2.支持箭头自定义图标及翻转动画

@vercel vercel bot temporarily deployed to Preview – zarm May 28, 2023 06:58 Inactive
@vercel vercel bot temporarily deployed to Preview – zarm May 29, 2023 02:00 Inactive
@JeromeLin
Copy link
Collaborator

还需要改进一下:

  1. 不同 Dropdown.Item 的 Popup 弹层内容需要共用,而不是每个 Dropdown.Item 都有一个Popup,这样在切换的时候不会重新执行下拉动画。
  2. 默认展示内置的箭头图标,且由 arrow 包裹(默认带动画),arrow 属性仅替换图标内容。
  3. direction 属性似乎逻辑反了,“向上展开” 例子为何是 direction="bottom"

@vercel vercel bot temporarily deployed to Preview – zarm May 31, 2023 07:27 Inactive
@JunIce
Copy link
Contributor Author

JunIce commented May 31, 2023

还需要改进一下:

  1. 不同 Dropdown.Item 的 Popup 弹层内容需要共用,而不是每个 Dropdown.Item 都有一个Popup,这样在切换的时候不会重新执行下拉动画。
  2. 默认展示内置的箭头图标,且由 arrow 包裹(默认带动画),arrow 属性仅替换图标内容。
  3. direction 属性似乎逻辑反了,“向上展开” 例子为何是 direction="bottom"

direction 重新赋值为updown, 这样语义比较清楚
Popup提升到Dropdown内部, Transition实现切换动画, 你看看是否符合预期

@vercel vercel bot temporarily deployed to Preview – zarm May 31, 2023 09:24 Inactive
@JeromeLin
Copy link
Collaborator

JeromeLin commented Jun 1, 2023

还需要改进一下:

  1. 不同 Dropdown.Item 的 Popup 弹层内容需要共用,而不是每个 Dropdown.Item 都有一个Popup,这样在切换的时候不会重新执行下拉动画。
  2. 默认展示内置的箭头图标,且由 arrow 包裹(默认带动画),arrow 属性仅替换图标内容。
  3. direction 属性似乎逻辑反了,“向上展开” 例子为何是 direction="bottom"

direction 重新赋值为updown, 这样语义比较清楚 Popup提升到Dropdown内部, Transition实现切换动画, 你看看是否符合预期

1.箭头方向反了,默认朝下
2.例子的内容再丰富一点,比如带多行文本、表单、列表项等,支持内部按钮关闭Dropdown弹层

@vercel vercel bot temporarily deployed to Preview – zarm June 4, 2023 07:17 Inactive
@vercel vercel bot temporarily deployed to Preview – zarm June 4, 2023 09:44 Inactive
@JeromeLin
Copy link
Collaborator

image
层叠顺序有点问题,先打开的内容要被后打开的遮盖

@JeromeLin
Copy link
Collaborator

这样处理层级会有问题,内部的Picker组件的弹层就在Dropdown下面了。
看起来和Picker类似,只需要定义Dropdown的 z-index 为一个固定值,靠堆叠顺序,节点在后的,盖在前一个上面就行了,不需要额外操作,参考下Picker吧

@JunIce
Copy link
Contributor Author

JunIce commented Jun 7, 2023

@JeromeLin 看了一下仅靠生成节点的顺序可能还不行,现在popup不是强制destroy,会出现之前的节点被再次打开,所以会出现盖不住的情况
和Picker还不太一样,Picker同时只会有一个打开
如果改成强制destroy可以解决,你看行不行

@JeromeLin
Copy link
Collaborator

@JeromeLin 看了一下仅靠生成节点的顺序可能还不行,现在popup不是强制destroy,会出现之前的节点被再次打开,所以会出现盖不住的情况 和Picker还不太一样,Picker同时只会有一个打开 如果改成强制destroy可以解决,你看行不行

干脆限制只允许同时打开一个吧,把之前开的都关了

@JeromeLin
Copy link
Collaborator

@JunIce 激活菜单的例子有点问题

| :--------------- | :-------------------- | :----- | :--------------------------- |
| activeKey | number \| string | - | 激活的 Item Key |
| defaultActiveKey | number \| string | - | 默认激活的 Item Key |
| direction | 'up' \| 'down' | 'down' | 默认下拉方向 |
Copy link
Collaborator

Choose a reason for hiding this comment

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

默认下拉方向 => 下拉方向

| animationType | string | - | 菜单动画类型,可选值 `fade`、`door`、`flip`、`rotate`、`zoom`、`zoom-fade`、`move-up`、 `move-down`、`move-left`、`move-right`、`slide-up`、`slide-down`、`slide-left`、`slide-right` |
| animationDuration | number | - | 动画执行时间(单位:毫秒) |
| popupClassName | string | - | 弹出层样式名 |
| zIndex | number | - | 下拉层级,默认从1200开始累加 |
Copy link
Collaborator

Choose a reason for hiding this comment

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

删除 zIndex 属性

interface CompoundedComponent
extends React.ForwardRefExoticComponent<DropdownProps & React.RefAttributes<DropdownInstance>> {
Item: typeof DropdownItem;
_zIndex: number;
Copy link
Collaborator

Choose a reason for hiding this comment

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

删除 zIndex 属性

@JeromeLin
Copy link
Collaborator

JeromeLin commented Jun 15, 2023

@JunIce 默认激活 的示例有问题。
另外分支切到next吧,发布到V4

Comment on lines +218 to +222
if (direction === 'down') {
styleOffset.top = `${offset.current}px`;
} else {
styleOffset.bottom = `${offset.current}px`;
styleOffset.height = `auto`;
Copy link
Contributor

Choose a reason for hiding this comment

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

resize/orientationchange 后,容器位置可以重新计算下

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

@JunIce
Copy link
Contributor Author

JunIce commented Jun 15, 2023

@JunIce 默认激活 的示例有问题。 另外分支切到next吧,发布到V4

收到

@JunIce JunIce closed this Jun 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants