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

fix: interacting with footer components closes the date selection panel #683

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Yuiai01
Copy link
Contributor

@Yuiai01 Yuiai01 commented Oct 7, 2023

close ant-design/ant-design#33125

目前还有一些问题:
1、input 需要设置 stopPropagation 才能聚焦,是否要在 footer 处集成;
2、select 等需要点击临时元素的,需要设置 getPopupContainer={(e) => e.parentElement}。

@vercel
Copy link

vercel bot commented Oct 7, 2023

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

Name Status Preview Comments Updated (UTC)
picker ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 7, 2023 2:55pm

@codecov
Copy link

codecov bot commented Oct 7, 2023

Codecov Report

Merging #683 (4915ccf) into master (f6c6803) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 4915ccf differs from pull request most recent head 9b843e9. Consider uploading reports for the commit 9b843e9 to get more accurate results

@@           Coverage Diff           @@
##           master     #683   +/-   ##
=======================================
  Coverage   98.94%   98.94%           
=======================================
  Files          56       56           
  Lines        2457     2460    +3     
  Branches      734      735    +1     
=======================================
+ Hits         2431     2434    +3     
  Misses         24       24           
  Partials        2        2           
Files Coverage Δ
src/Picker.tsx 100.00% <ø> (ø)
src/RangePicker.tsx 99.29% <ø> (ø)
src/hooks/usePickerInput.ts 98.76% <100.00%> (+0.04%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@Yuiai01
Copy link
Contributor Author

Yuiai01 commented Oct 7, 2023

目前还有一些问题:
1、input 需要设置 stopPropagation 才能聚焦,是否要在 footer 处集成;
2、select 等需要点击临时元素的,需要设置 getPopupContainer={(e) => e.parentElement}

@@ -326,6 +327,7 @@ function InnerPicker<DateType>(props: PickerProps<DateType>) {
const [inputProps, { focused, typing }] = usePickerInput({
blurToCancel: needConfirmButton,
changeOnBlur,
hasExtraFooter: !!renderExtraFooter,
Copy link
Contributor

Choose a reason for hiding this comment

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

usePickerInput 可以直接接收 renderExtraFooter 吗,不需要用 hasExtraFooter

Copy link
Contributor Author

Choose a reason for hiding this comment

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

里面主要用来做逻辑判断应该用不到 renderExtraFooter 方法,因此我在外面判断是否存在。

onSubmit: () => void | boolean;
onCancel: () => void;
onFocus?: React.FocusEventHandler<HTMLInputElement>;
onBlur?: React.FocusEventHandler<HTMLInputElement>;
}): [React.DOMAttributes<HTMLInputElement>, { focused: boolean; typing: boolean }] {
const [typing, setTyping] = useState(false);
const [focused, setFocused] = useState(false);
const [outside, setOutside] = useState(true);
Copy link
Contributor

Choose a reason for hiding this comment

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

这个改变需要 render 吗,可以用 useRef 吗

Copy link
Contributor Author

Choose a reason for hiding this comment

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

对,只需要阻塞一下 blur。

@@ -152,6 +155,11 @@ export default function usePickerInput({
const target = getTargetFromEvent(e);
const clickedOutside = isClickOutside(target);

if (hasExtraFooter) {
Copy link
Member

Choose a reason for hiding this comment

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

这种零碎逻辑太多了不好,感觉是写个 RFC 让 ConfigProvider 支持关闭逻辑。例如默认是 blur 关闭,也可以设置成点击外部关闭。这样就不管你是哪个组件,都是一样的逻辑。

Copy link
Contributor

Choose a reason for hiding this comment

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

image

footer 可以是按钮,需要点击后关闭 Picker,如果是 Input 就不需要关闭,最终还是要在 Picker 支持 props 控制是否可以关闭吧?

Copy link
Contributor

@crazyair crazyair Oct 10, 2023

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.

image

footer 可以是按钮,需要点击后关闭 Picker,如果是 Input 就不需要关闭,最终还是要在 Picker 支持 props 控制是否可以关闭吧?

嗯嗯

Copy link
Contributor

Choose a reason for hiding this comment

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

另外,点击 renderExtraFooter 位置元素,应该不属于 blur 啊?也没有离开 Picker 啊?为什么会触发 blur 呢?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

触发了 input 的 blur 。

Copy link
Contributor

Choose a reason for hiding this comment

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

还有 Select 的自定义 render,也需要阻止冒泡。按理说点击自定义添加的按钮或者 Input,应该不属于 Select 的选中,不应该关闭 Select 的下拉框的

Copy link
Contributor Author

Choose a reason for hiding this comment

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

对的,所以这里可以整理一个 RFC 出来。

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.

[4.0] 操作DatePicker.RangePicker的renderExtraFooter上定义的控件时,面板会关闭
3 participants