Skip to content

fix(site): update Spinner component to avoid UI edge cases #16497

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

Closed
wants to merge 23 commits into from
Closed
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
8da41df
refactor: clean up existing code
Parkreiner Feb 7, 2025
c9841f2
fix: update API for component
Parkreiner Feb 7, 2025
2f356bc
wip: get majority of component updated
Parkreiner Feb 7, 2025
87fd0c2
chore: get initial version of spinner update in place
Parkreiner Feb 7, 2025
e9b0e5e
docs: switch main comment to JSDoc
Parkreiner Feb 7, 2025
150370d
fix: update state definitions
Parkreiner Feb 7, 2025
b2870a6
fix: add delay safety nets
Parkreiner Feb 7, 2025
90df622
refactor: clean up current code
Parkreiner Feb 7, 2025
d082834
docs: fix typo
Parkreiner Feb 7, 2025
e094eb0
fix: remove infinite render loops
Parkreiner Feb 7, 2025
c8ba1d0
fix: more adjustments to avoid infinite re-renders
Parkreiner Feb 7, 2025
b0d020e
refactor: split up nasty state logic to make main component more read…
Parkreiner Feb 7, 2025
4560c24
fix: more render stabilization
Parkreiner Feb 7, 2025
7a668d2
docs: update comments for clarity
Parkreiner Feb 7, 2025
a69d576
refactor: clean up code more
Parkreiner Feb 7, 2025
3e0db15
docs: rewrite docs for clarity again
Parkreiner Feb 7, 2025
3567280
docs: remove typo
Parkreiner Feb 7, 2025
6ad1cf2
docs: add link reference for cursed React techniques
Parkreiner Feb 7, 2025
fc44cdb
fix: add edge-case protection for certain delay values
Parkreiner Feb 7, 2025
638ccf5
fix: make first effort towards preserving state when content for load…
Parkreiner Feb 10, 2025
ce8e555
refactor: make it harder to misuse render cache
Parkreiner Feb 10, 2025
3b058df
refactor: clean up current implementation
Parkreiner Feb 11, 2025
815c71d
docs: fix links
Parkreiner Feb 11, 2025
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
Prev Previous commit
Next Next commit
refactor: clean up current code
  • Loading branch information
Parkreiner committed Feb 7, 2025
commit 90df622d0d4115bd3249feabb3434a675d35d4de
19 changes: 10 additions & 9 deletions site/src/components/Spinner/Spinner.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -90,30 +90,31 @@ export const Spinner: FC<SpinnerProps> = ({
// only bails out of redundant state updates if they happen outside of a
// render. Inside a render, if you keep calling a state dispatch, you will
// get an infinite render loop, no matter what the state value is.
const [delayDone, setDelayDone] = useState(safeDelay === 0);
if (delayDone && !loading) {
setDelayDone(false);
const [delayExpired, setDelayExpired] = useState(safeDelay === 0);
if (delayExpired && !loading) {
setDelayExpired(false);
}
if (!delayDone && loading && safeDelay === 0) {
setDelayDone(true);
if (!delayExpired && safeDelay === 0) {
setDelayExpired(true);
}
useEffect(() => {
if (safeDelay === 0) {
return;
}

const delayId = window.setTimeout(() => {
setDelayDone(true);
setDelayExpired(true);
}, safeDelay);
return () => window.clearTimeout(delayId);
}, [safeDelay]);

/**
* @todo Figure out if this conditional logic can ever cause a component to
* lose state. I would hope not, since the children prop is the same in both
* cases, but I need to test this out
* lose state when showSpinner flips from false to true while
* unmountedWhileLoading is false. I would hope not, since the children prop
* is the same in both cases, but I need to test this out
*/
const showSpinner = delayDone && loading;
const showSpinner = loading && delayExpired;
if (!showSpinner) {
return children;
}
Expand Down
Loading