Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitfcc8b9e

Browse files
authored
fix: prevent workspace search bar text from getting garbled (#9703)
* chore: Reorganize hook calls for useWorkspacesFilter* refactor: Clean up some filter logic* refactor: Create debounce utility hooks* docs: Clean up comments for clarity* fix: Update focus logic to apply for any inner focus* fix: Add onBlur behavior for state syncs* chore: Add progress for debounce test* chore: Finish tests for debounce hooks* docs: Add file description and warning
1 parentb104e0e commitfcc8b9e

File tree

5 files changed

+374
-42
lines changed

5 files changed

+374
-42
lines changed

‎site/src/components/Filter/filter.tsx‎

Lines changed: 43 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,11 @@ import {
2020
}from"api/errors";
2121
import{useFilterMenu}from"./menu";
2222
import{BaseOption}from"./options";
23-
importdebouncefrom"just-debounce-it";
2423
importMenuListfrom"@mui/material/MenuList";
2524
import{Loader}from"components/Loader/Loader";
2625
importDividerfrom"@mui/material/Divider";
2726
importOpenInNewOutlinedfrom"@mui/icons-material/OpenInNewOutlined";
27+
import{useDebouncedFunction}from"hooks/debounce";
2828

2929
exporttypePresetFilter={
3030
name:string;
@@ -58,7 +58,7 @@ export const useFilter = ({
5858
}
5959
};
6060

61-
constdebounceUpdate=debounce(
61+
const{debounced:debounceUpdate, cancelDebounce}=useDebouncedFunction(
6262
(values:string|FilterValues)=>update(values),
6363
500,
6464
);
@@ -69,6 +69,7 @@ export const useFilter = ({
6969
query,
7070
update,
7171
debounceUpdate,
72+
cancelDebounce,
7273
values,
7374
used,
7475
};
@@ -130,17 +131,7 @@ export const MenuSkeleton = () => (
130131
<BaseSkeletonsx={{minWidth:200,flexShrink:0}}/>
131132
);
132133

133-
exportconstFilter=({
134-
filter,
135-
isLoading,
136-
error,
137-
skeleton,
138-
options,
139-
learnMoreLink,
140-
learnMoreLabel2,
141-
learnMoreLink2,
142-
presets,
143-
}:{
134+
typeFilterProps={
144135
filter:ReturnType<typeofuseFilter>;
145136
skeleton:ReactNode;
146137
isLoading:boolean;
@@ -150,20 +141,42 @@ export const Filter = ({
150141
error?:unknown;
151142
options?:ReactNode;
152143
presets:PresetFilter[];
153-
})=>{
154-
constshouldDisplayError=hasError(error)&&isApiValidationError(error);
155-
consthasFilterQuery=filter.query!=="";
156-
const[searchQuery,setSearchQuery]=useState(filter.query);
157-
constinputRef=useRef<HTMLInputElement>(null);
144+
};
158145

146+
exportconstFilter=({
147+
filter,
148+
isLoading,
149+
error,
150+
skeleton,
151+
options,
152+
learnMoreLink,
153+
learnMoreLabel2,
154+
learnMoreLink2,
155+
presets,
156+
}:FilterProps)=>{
157+
// Storing local copy of the filter query so that it can be updated more
158+
// aggressively without re-renders rippling out to the rest of the app every
159+
// single time. Exists for performance reasons - not really a good way to
160+
// remove this; render keys would cause the component to remount too often
161+
const[queryCopy,setQueryCopy]=useState(filter.query);
162+
consttextboxInputRef=useRef<HTMLInputElement>(null);
163+
164+
// Conditionally re-syncs the parent and local filter queries
159165
useEffect(()=>{
160-
// We don't want to update this while the user is typing something or has the focus in the input
161-
constisFocused=document.activeElement===inputRef.current;
162-
if(!isFocused){
163-
setSearchQuery(filter.query);
166+
consthasSelfOrInnerFocus=
167+
textboxInputRef.current?.contains(document.activeElement)??false;
168+
169+
// This doesn't address all state sync issues - namely, what happens if the
170+
// user removes focus just after this synchronizing effect fires. Also need
171+
// to rely on onBlur behavior as an extra safety measure
172+
if(!hasSelfOrInnerFocus){
173+
setQueryCopy(filter.query);
164174
}
165175
},[filter.query]);
166176

177+
constshouldDisplayError=hasError(error)&&isApiValidationError(error);
178+
consthasFilterQuery=filter.query!=="";
179+
167180
return(
168181
<Box
169182
sx={{
@@ -198,12 +211,17 @@ export const Filter = ({
198211
"aria-label":"Filter",
199212
name:"query",
200213
placeholder:"Search...",
201-
value:searchQuery,
202-
ref:inputRef,
214+
value:queryCopy,
215+
ref:textboxInputRef,
203216
onChange:(e)=>{
204-
setSearchQuery(e.target.value);
217+
setQueryCopy(e.target.value);
205218
filter.debounceUpdate(e.target.value);
206219
},
220+
onBlur:()=>{
221+
if(queryCopy!==filter.query){
222+
setQueryCopy(filter.query);
223+
}
224+
},
207225
sx:{
208226
borderRadius:"6px",
209227
borderTopLeftRadius:0,

‎site/src/hooks/debounce.test.ts‎

Lines changed: 193 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,193 @@
1+
import{renderHook}from"@testing-library/react";
2+
import{useDebouncedFunction,useDebouncedValue}from"./debounce";
3+
4+
beforeAll(()=>{
5+
jest.useFakeTimers();
6+
jest.spyOn(global,"setTimeout");
7+
});
8+
9+
afterAll(()=>{
10+
jest.useRealTimers();
11+
jest.clearAllMocks();
12+
});
13+
14+
// Most UI tests should be structure from the user's experience, but just
15+
// because these are more abstract, general-purpose hooks, it seemed harder to
16+
// do that. Had to bring in some mocks
17+
functionrenderDebouncedValue<T=unknown>(value:T,time:number){
18+
returnrenderHook(
19+
({ value, time}:{value:T;time:number})=>{
20+
returnuseDebouncedValue(value,time);
21+
},
22+
{
23+
initialProps:{ value, time},
24+
},
25+
);
26+
}
27+
28+
functionrenderDebouncedFunction<Argsextendsunknown[]>(
29+
callbackArg:(...args:Args)=>void|Promise<void>,
30+
time:number,
31+
){
32+
returnrenderHook(
33+
({ callback, time}:{callback:typeofcallbackArg;time:number})=>{
34+
returnuseDebouncedFunction<Args>(callback,time);
35+
},
36+
{
37+
initialProps:{callback:callbackArg, time},
38+
},
39+
);
40+
}
41+
42+
describe(`${useDebouncedValue.name}`,()=>{
43+
it("Should immediately return out the exact same value (by reference) on mount",()=>{
44+
constvalue={};
45+
const{ result}=renderDebouncedValue(value,2000);
46+
47+
expect(result.current).toBe(value);
48+
expect.hasAssertions();
49+
});
50+
51+
it("Should not immediately resync state as the hook re-renders with new value argument",async()=>{
52+
letvalue=0;
53+
consttime=5000;
54+
55+
const{ result, rerender}=renderDebouncedValue(value,time);
56+
expect(result.current).toEqual(0);
57+
58+
for(leti=1;i<=5;i++){
59+
setTimeout(()=>{
60+
value++;
61+
rerender({ value, time});
62+
},i*100);
63+
}
64+
65+
awaitjest.advanceTimersByTimeAsync(time-100);
66+
expect(result.current).toEqual(0);
67+
expect.hasAssertions();
68+
});
69+
70+
it("Should resync after specified milliseconds pass with no change to arguments",async()=>{
71+
constinitialValue=false;
72+
consttime=5000;
73+
74+
const{ result, rerender}=renderDebouncedValue(initialValue,time);
75+
expect(result.current).toEqual(false);
76+
77+
rerender({value:!initialValue, time});
78+
awaitjest.runAllTimersAsync();
79+
80+
expect(result.current).toEqual(true);
81+
expect.hasAssertions();
82+
});
83+
});
84+
85+
describe(`${useDebouncedFunction.name}`,()=>{
86+
describe("hook",()=>{
87+
it("Should provide stable function references across re-renders",()=>{
88+
consttime=5000;
89+
const{ result, rerender}=renderDebouncedFunction(jest.fn(),time);
90+
91+
const{debounced:oldDebounced,cancelDebounce:oldCancel}=
92+
result.current;
93+
94+
rerender({callback:jest.fn(), time});
95+
const{debounced:newDebounced,cancelDebounce:newCancel}=
96+
result.current;
97+
98+
expect(oldDebounced).toBe(newDebounced);
99+
expect(oldCancel).toBe(newCancel);
100+
expect.hasAssertions();
101+
});
102+
103+
it("Resets any pending debounces if the timer argument changes",async()=>{
104+
consttime=5000;
105+
letcount=0;
106+
constincrementCount=()=>{
107+
count++;
108+
};
109+
110+
const{ result, rerender}=renderDebouncedFunction(
111+
incrementCount,
112+
time,
113+
);
114+
115+
result.current.debounced();
116+
rerender({callback:incrementCount,time:time+1});
117+
118+
awaitjest.runAllTimersAsync();
119+
expect(count).toEqual(0);
120+
expect.hasAssertions();
121+
});
122+
});
123+
124+
describe("debounced function",()=>{
125+
it("Resolve the debounce after specified milliseconds pass with no other calls",async()=>{
126+
letvalue=false;
127+
const{ result}=renderDebouncedFunction(()=>{
128+
value=!value;
129+
},100);
130+
131+
result.current.debounced();
132+
133+
awaitjest.runOnlyPendingTimersAsync();
134+
expect(value).toBe(true);
135+
expect.hasAssertions();
136+
});
137+
138+
it("Always uses the most recent callback argument passed in (even if it switches while a debounce is queued)",async()=>{
139+
letcount=0;
140+
consttime=500;
141+
142+
const{ result, rerender}=renderDebouncedFunction(()=>{
143+
count=1;
144+
},time);
145+
146+
result.current.debounced();
147+
rerender({
148+
callback:()=>{
149+
count=9999;
150+
},
151+
time,
152+
});
153+
154+
awaitjest.runAllTimersAsync();
155+
expect(count).toEqual(9999);
156+
expect.hasAssertions();
157+
});
158+
159+
it("Should reset the debounce timer with repeated calls to the method",async()=>{
160+
letcount=0;
161+
const{ result}=renderDebouncedFunction(()=>{
162+
count++;
163+
},2000);
164+
165+
for(leti=0;i<10;i++){
166+
setTimeout(()=>{
167+
result.current.debounced();
168+
},i*100);
169+
}
170+
171+
awaitjest.runAllTimersAsync();
172+
expect(count).toBe(1);
173+
expect.hasAssertions();
174+
});
175+
});
176+
177+
describe("cancelDebounce function",()=>{
178+
it("Should be able to cancel a pending debounce",async()=>{
179+
letcount=0;
180+
const{ result}=renderDebouncedFunction(()=>{
181+
count++;
182+
},2000);
183+
184+
const{ debounced, cancelDebounce}=result.current;
185+
debounced();
186+
cancelDebounce();
187+
188+
awaitjest.runAllTimersAsync();
189+
expect(count).toEqual(0);
190+
expect.hasAssertions();
191+
});
192+
});
193+
});

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp