Skip to content

feat(module:resizable): support for multiple cursor types #8042

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

Merged
merged 1 commit into from
Sep 18, 2023

Conversation

HyperLife1119
Copy link
Collaborator

@HyperLife1119 HyperLife1119 commented Aug 2, 2023

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Application (the showcase website) / infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

@HyperLife1119 HyperLife1119 requested a review from hsuanxyz as a code owner August 2, 2023 07:38
@codecov
Copy link

codecov bot commented Aug 2, 2023

Codecov Report

Merging #8042 (6637357) into master (fa0312a) will decrease coverage by 0.02%.
The diff coverage is 71.42%.

❗ Current head 6637357 differs from pull request most recent head 084109d. Consider uploading reports for the commit 084109d to get more accurate results

@@            Coverage Diff             @@
##           master    #8042      +/-   ##
==========================================
- Coverage   91.64%   91.63%   -0.02%     
==========================================
  Files         515      515              
  Lines       17640    17637       -3     
  Branches     2790     2790              
==========================================
- Hits        16167    16161       -6     
- Misses       1175     1178       +3     
  Partials      298      298              
Files Changed Coverage Δ
components/resizable/resizable.directive.ts 94.90% <ø> (-0.42%) ⬇️
components/resizable/resize-handle.component.ts 84.00% <20.00%> (-16.00%) ⬇️
components/resizable/demo/grid.ts 100.00% <100.00%> (ø)
components/resizable/resize-handles.component.ts 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

@hsuanxyz
Copy link
Member

hsuanxyz commented Aug 2, 2023

The cursor is the same as the window resize cursor. for this change, I think it should only apply when the cursor is at the edge of the resizeable box. like [Grid] and [Layout] in the demos.

@HyperLife1119
Copy link
Collaborator Author

HyperLife1119 commented Aug 2, 2023

我们可以为 nz-resizable 增加一个 input 用于指示当前的 resize 场景吗:

type NzScene = 'window' | 'layout';
<nz-resizable nzScene="layout"></nz-resizable>
  • layout 表示当前 resize 场景为布局型的,光标将使用 row/col-resize
  • window 表示当前 resize 场景为窗口型的,光标将使用 window resize cursor

让我知道你的想法 :)

@hsuanxyz
Copy link
Member

hsuanxyz commented Aug 2, 2023

我觉得可以直接叫 nzCursorType: "grid" | "window" ,功能自身没有 Scene 那么宽的职责。另外可以再把 css 选择器更新到文档末尾,就像 ".nz-resizable-handle-cursor-type-grid .nz-resizable-handle-cursor-type-window",方便用户覆盖样式

@HyperLife1119 HyperLife1119 force-pushed the feat/resizable-cursor branch from 32149be to 0560b5d Compare August 2, 2023 11:03
@HyperLife1119 HyperLife1119 changed the title feat(module:resizable): better horizontal/vertical drag cursor feat(module:resizable): support for modifying the cursor type Aug 2, 2023
@HyperLife1119 HyperLife1119 force-pushed the feat/resizable-cursor branch from 0560b5d to d011a5f Compare August 2, 2023 11:09
@@ -93,4 +103,18 @@ export class NzResizeHandleComponent implements OnInit {
});
});
}

@HostListener('pointerdown')
Copy link
Member

Choose a reason for hiding this comment

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

为什么我们不用 mousedown

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

PointerEvent 同时支持 mouse/touch 等多种指针事件

Copy link
Member

Choose a reason for hiding this comment

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

那也许我们可以试试 Element.setPointerCapture() 这样就不用 getComputedStyle

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

可以的,我在 Chrome/Firefox for Mac 下测试,使用 Element.setPointerCapture(),我们甚至不需要为 doc.body 设置 user-select: none 样式,也不再需要 :not(.@{resizable-prefix-cls}-resizing) 条件选择器

Copy link
Collaborator

Choose a reason for hiding this comment

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

@HyperLife1119 这里会导致 nz-resize-handle 中的子组件的 onClick 事件被吞掉,需要多次点击才能触发
https://stackblitz.com/edit/angular-41b5ra?file=src%2Fapp%2Fapp.component.ts

@HyperLife1119 HyperLife1119 force-pushed the feat/resizable-cursor branch from d3145e3 to 4590480 Compare August 3, 2023 04:22
},
providers: [NzDestroyService]
})
export class NzResizeHandleComponent implements OnInit {
@Input() nzDirection: NzResizeDirection = 'bottomRight';
@Output() readonly nzMouseDown = new EventEmitter<NzResizeHandleMouseDownEvent>();

protected readonly resizableDirective = inject(NzResizableDirective);
Copy link
Collaborator Author

@HyperLife1119 HyperLife1119 Aug 3, 2023

Choose a reason for hiding this comment

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

由于 nz-resize-handlenz-resize-handles 都是 OnPush 的,只有在它们的 Inputs 发生变化时才会进行变更检测,这将导致 nz-resize-handle 无法响应 [nz-resizable] 指令的 nzCursorType 变更。

一些可能的解决办法:

  1. nz-resize-handlenz-resize-handles 组件调整为 ChangeDetectionStrategy.Default
    我认为这不会引起太大的性能问题,因为它们的父组件([nz-resizable])是 OnPush 的,也就是说,它们的变更检测时机将受限于父组件:只有当父组件进行变更检测后,子组件才有机会进行变更检测

  2. nz-resize-handlenz-resize-handles 组件添加 nzCurosrType input,逐层传递

  3. 使用 ngDoCheck,在 nz-resize-handle 组件内部实现自定义变更检测策略

我更倾向于使用第一种方式或第三种办法,你认为呢?

Copy link
Member

Choose a reason for hiding this comment

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

Input 放 nz-resize-handle[nzCursorType] 上呢,比如四周用 window 中间用 grid

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

那这样 nz-resize-handles 就得支持传递 8 个方向的 nzCursorType 了,这可能需要重构 nz-resize-handles[nzDirections] input 的类型,或新增另一个 input

Copy link
Member

Choose a reason for hiding this comment

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

改 nzDirections 的类型吧,这样实现也简单点,一路传下去就行了

@HyperLife1119 HyperLife1119 force-pushed the feat/resizable-cursor branch from 4590480 to cb9ba50 Compare August 5, 2023 16:05
@HyperLife1119 HyperLife1119 changed the title feat(module:resizable): support for modifying the cursor type feat(module:resizable): support for multiple cursor types Aug 5, 2023
@HyperLife1119 HyperLife1119 force-pushed the feat/resizable-cursor branch from cb9ba50 to 619afa5 Compare August 5, 2023 16:20
@HyperLife1119 HyperLife1119 force-pushed the feat/resizable-cursor branch from 619afa5 to 084109d Compare August 5, 2023 16:22
@HyperLife1119 HyperLife1119 requested a review from hsuanxyz August 5, 2023 16:48
Copy link
Member

@hsuanxyz hsuanxyz left a comment

Choose a reason for hiding this comment

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

LGTM

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.

5 participants