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: trigger onBlur does not require reset focused #980

Merged
merged 5 commits into from
Sep 14, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/BaseSelect.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,7 @@ const BaseSelect = React.forwardRef((props: BaseSelectProps, ref: React.Ref<Base
const triggerRef = React.useRef<RefTriggerProps>(null);
const selectorRef = React.useRef<RefSelectorProps>(null);
const listRef = React.useRef<RefOptionListProps>(null);
const blurRef = React.useRef<boolean>(false);

/** Used for component focused management */
const [mockFocused, setMockFocused, cancelSetMockFocused] = useDelayReset();
Expand Down Expand Up @@ -451,7 +452,8 @@ const BaseSelect = React.forwardRef((props: BaseSelectProps, ref: React.Ref<Base
setInnerOpen(false);
}

if (disabled) {
// After onBlur is triggered, the focused does not need to be reset
if (disabled && !blurRef.current) {
setMockFocused(false);
}
}, [disabled]);
Expand Down Expand Up @@ -561,8 +563,11 @@ const BaseSelect = React.forwardRef((props: BaseSelectProps, ref: React.Ref<Base
};

const onContainerBlur: React.FocusEventHandler<HTMLElement> = (...args) => {
blurRef.current = true;

setMockFocused(false, () => {
focusRef.current = false;
blurRef.current = false;
onToggleOpen(false);
});

Expand Down
37 changes: 35 additions & 2 deletions tests/focus.test.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { mount } from 'enzyme';
import React from 'react';
import { mount} from 'enzyme';
import React, { useState } from 'react';
MadCcc marked this conversation as resolved.
Show resolved Hide resolved
import { act } from 'react-dom/test-utils';
import Select from '../src';
import { fireEvent, render } from '@testing-library/react';

describe('Select.Focus', () => {
it('disabled should reset focused', () => {
Expand Down Expand Up @@ -31,6 +32,38 @@ describe('Select.Focus', () => {
jest.useRealTimers();
});

it('after onBlur is triggered the focused does not need to be reset', () => {
jest.useFakeTimers();

const Demo: React.FC = () => {
const [disabled, setDisabled] = useState(false);
return (
<>
<Select disabled={disabled} onBlur={() => setDisabled(true)} />
<button onClick={() => setDisabled(false)} />
</>
);
};

const { container } = render(<Demo />);

fireEvent.focus(container.querySelector('input'));
jest.runAllTimers();

// trigger disabled
fireEvent.blur(container.querySelector('input'));
jest.runAllTimers();

// reset disabled
fireEvent.click(container.querySelector('button'));

fireEvent.focus(container.querySelector('input'));
jest.runAllTimers();

expect(container.querySelector('.rc-select-focused')).toBeTruthy();
jest.useRealTimers();
Copy link
Member

Choose a reason for hiding this comment

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

这个用例感觉不对,上一步 focus input 之后这里必然是 truthy

Copy link
Contributor Author

Choose a reason for hiding this comment

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

最开始的 focus 有触发 onBlur 取消掉,后续重新 focus input 到这里应当是 truthy。这个问题是由于 disabled 重置了 focused 导致无法再次触发 focus,所以这里逻辑应该没问题。

Copy link
Member

@MadCcc MadCcc Sep 14, 2023

Choose a reason for hiding this comment

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

感觉上应该是判断 onFocus 事件有没有触发?focus 状态是有的,但是原问题是内部的 focus 状态错误地保留了导致没有触发 foucs 事件。

});

it('IE focus', () => {
jest.clearAllTimers();
jest.useFakeTimers();
Expand Down
Loading