-
Notifications
You must be signed in to change notification settings - Fork 16
refactor: Replace StateContextProvider with Zustand for state management (resolves #168, resolves #211). #224
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
Changes from all commits
47ffe61
9623f99
dec8596
1dff9fe
e865326
1bcb881
0e51fb5
40ce59b
35b341f
0a98905
69b0892
2b06efd
30487c6
c7cd652
7f4343d
87b2da3
f1d67f7
e7b7481
b7b4fef
9f6c256
5874893
ebb664c
af78846
d99deb6
b7d6bdf
f61cc09
fd77cff
0d5e948
75d203a
572b319
d59dd4a
50d3f27
d3b52dc
589d246
351757d
03552b6
c085441
81e7d2e
1443bb8
8d816d0
cec35ea
ff16262
68f9325
4b8c532
3f065a5
b0135b3
301ce91
84be9b7
8b2c8f6
2cc29f6
0bd4385
02f929d
4389632
46471ed
6695360
d5e5b55
72650b9
1b49af7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,19 +1,14 @@ | ||
import React, { | ||
useContext, | ||
useState, | ||
} from "react"; | ||
import React from "react"; | ||
|
||
import { | ||
LinearProgress, | ||
Stack, | ||
Textarea, | ||
} from "@mui/joy"; | ||
|
||
import {StateContext} from "../../../../../contexts/StateContextProvider"; | ||
import { | ||
QUERY_PROGRESS_VALUE_MAX, | ||
QueryArgs, | ||
} from "../../../../../typings/query"; | ||
import useQueryStore from "../../../../../stores/queryStore"; | ||
import useUiStore from "../../../../../stores/uiStore"; | ||
import {QUERY_PROGRESS_VALUE_MAX} from "../../../../../typings/query"; | ||
import {UI_ELEMENT} from "../../../../../typings/states"; | ||
import {isDisabled} from "../../../../../utils/states"; | ||
import ToggleIconButton from "./ToggleIconButton"; | ||
|
@@ -27,34 +22,29 @@ import "./QueryInputBox.css"; | |
* @return | ||
*/ | ||
const QueryInputBox = () => { | ||
const {queryProgress, startQuery, uiState} = useContext(StateContext); | ||
|
||
const [queryString, setQueryString] = useState<string>(""); | ||
const [isCaseSensitive, setIsCaseSensitive] = useState<boolean>(false); | ||
const [isRegex, setIsRegex] = useState<boolean>(false); | ||
|
||
const handleQuerySubmit = (newArgs: Partial<QueryArgs>) => { | ||
startQuery({ | ||
isCaseSensitive: isCaseSensitive, | ||
isRegex: isRegex, | ||
queryString: queryString, | ||
...newArgs, | ||
}); | ||
}; | ||
const isCaseSensitive = useQueryStore((state) => state.queryIsCaseSensitive); | ||
const isRegex = useQueryStore((state) => state.queryIsRegex); | ||
const querystring = useQueryStore((state) => state.queryString); | ||
const setQueryIsCaseSensitive = useQueryStore((state) => state.setQueryIsCaseSensitive); | ||
const setQueryIsRegex = useQueryStore((state) => state.setQueryIsRegex); | ||
const setQueryString = useQueryStore((state) => state.setQueryString); | ||
const queryProgress = useQueryStore((state) => state.queryProgress); | ||
const startQuery = useQueryStore((state) => state.startQuery); | ||
const uiState = useUiStore((state) => state.uiState); | ||
|
||
const handleQueryInputChange = (ev: React.ChangeEvent<HTMLTextAreaElement>) => { | ||
setQueryString(ev.target.value); | ||
handleQuerySubmit({queryString: ev.target.value}); | ||
startQuery(); | ||
}; | ||
Comment on lines
35
to
38
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Running a query on every keystroke may exhaust the worker
Would you like a sample debounce implementation? |
||
|
||
const handleCaseSensitivityButtonClick = () => { | ||
handleQuerySubmit({isCaseSensitive: !isCaseSensitive}); | ||
setIsCaseSensitive(!isCaseSensitive); | ||
setQueryIsCaseSensitive(!isCaseSensitive); | ||
startQuery(); | ||
}; | ||
|
||
const handleRegexButtonClick = () => { | ||
handleQuerySubmit({isRegex: !isRegex}); | ||
setIsRegex(!isRegex); | ||
setQueryIsRegex(!isRegex); | ||
startQuery(); | ||
}; | ||
Comment on lines
40
to
48
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Follow project guideline: avoid The coding-guidelines section for setQueryIsCaseSensitive(!isCaseSensitive);
...
setQueryIsRegex(!isRegex); Suggested fix: -setQueryIsCaseSensitive(!isCaseSensitive);
+setQueryIsCaseSensitive(false == isCaseSensitive);
-setQueryIsRegex(!isRegex);
+setQueryIsRegex(false == isRegex); This also keeps the style consistent across the codebase. |
||
|
||
const isQueryInputBoxDisabled = isDisabled(uiState, UI_ELEMENT.QUERY_INPUT_BOX); | ||
|
@@ -66,6 +56,7 @@ const QueryInputBox = () => { | |
maxRows={7} | ||
placeholder={"Search"} | ||
size={"sm"} | ||
value={querystring} | ||
endDecorator={ | ||
<Stack | ||
direction={"row"} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Consider selector batching to avoid unnecessary re-renders
Each
useQueryStore
/useUiStore
call subscribes the component to a separate slice of state. Whenever any of those slices changes the component re-renders, which – with eight selectors – can become chatty.Two small tweaks improve performance without losing readability:
Advantages
• One subscription instead of eight.
• With
shallow
equality the component only re-renders when one of the selected values actually changes.You can pull
shallow
from"zustand/shallow"
.📝 Committable suggestion