react-component / select

React Select
https://select.react-component.now.sh/
MIT License
892 stars 452 forks source link

fix: fix weird behavior when using dynamic maxCountTag #943

Closed yangxiuxiu1115 closed 1 year ago

yangxiuxiu1115 commented 1 year ago

fix Weird behavior when using dynamic maxCountTag. #42551

我尝试去解决该问题,以下是我对该bug出现的缘由和涉及的知识,如有错误可以及时与我交流

出现原因

在issue中已经提供了复现的步骤,问题所在在于 onDropdownVisibleChange 被错误执行两次。问题出现缘由在于 maxTagCount,当 maxTagCount 是动态变化时,select 组件会进行一次页面更新。react所采用的是合成事件机制,而 select 组件因为需要实现点击其他区域时关闭 select 的操作,在 window 上绑定了一个 mousedown 事件。而在点击select组件输入框时,使用react合成事件onMouseDown,该事件会将select组件展开。而合成事件的onMouseDown会先于windows上的mousedown事件执行。问题出在于合成事件先执行时会展开select组件,随后maxTagCount发生变化,会产生一次回流更新(我不太清楚为什么会产生一次更新),更新会导致原先的event.target被移除,导致后续出发window上的mousedown时不能正确判断元素是在select组件之内,因此会将select组件关闭。

解决思路

知道了问题出现的原因后,解决问题也有跟多方案。在这里采用了将 mousedown 提前至捕获阶段执行的方案。

vercel[bot] commented 1 year ago

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

Name Status Preview Comments Updated (UTC)
select ❌ Failed (Inspect) Jun 10, 2023 9:52am
yoyo837 commented 1 year ago

能添加test case 覆盖到这个问题吗?

codecov[bot] commented 1 year ago

Codecov Report

Merging #943 (31b63bf) into master (7940e17) will decrease coverage by 0.01%. The diff coverage is 100.00%.

:exclamation: Current head 31b63bf differs from pull request most recent head d1526b9. Consider uploading reports for the commit d1526b9 to get more accurate results

@@            Coverage Diff             @@
##           master     #943      +/-   ##
==========================================
- Coverage   99.63%   99.63%   -0.01%     
==========================================
  Files          37       37              
  Lines        1356     1352       -4     
  Branches      393      365      -28     
==========================================
- Hits         1351     1347       -4     
  Misses          4        4              
  Partials        1        1              
Impacted Files Coverage Δ
src/hooks/useSelectTriggerControl.ts 94.44% <100.00%> (ø)

... and 1 file with indirect coverage changes

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

yangxiuxiu1115 commented 1 year ago

能添加test case 覆盖到这个问题吗?

我很乐意会它添加对应的 test case,我尝试了许多次,可能是因为我对 jest 并没有那么了解的原因,我无法正确写出对应的 test case,能给我一些帮助吗。

yangxiuxiu1115 commented 1 year ago

这是我尝试写的 test case,但是它好像并不符合预期

it('should be opened correctly when using dynamic maxCountTag and onDropdownVisibleChange', async () => {
const onDropdownVisibleChange = jest.fn();
const Test = () => {
const [isOpen, setIsOpen] = React.useState(false);
return (
<Select
mode="multiple"
open={isOpen}
onDropdownVisibleChange={(o) => {
setIsOpen(o);
onDropdownVisibleChange();
}}
value={['1', '2']}
maxTagCount={isOpen ? 0 : 'responsive'}

<Option value="1">1</Option>
<Option value="2">2</Option>
<Option value="3">3</Option>
<Option value="4">4</Option>
</Select>
);
};
const wrapper = mount(<Test />);
wrapper.find('.rc-select-selection-overflow').simulate('mousedown');
expectOpen(wrapper);
expect(onDropdownVisibleChange).toBeCalledTimes(1);
});