Skip to content

fix: Typing in notebooks is laggy #1977

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.

Sign up for GitHub

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

Merged
mofojed merged 8 commits into from
May 2, 2024
Merged

fix: Typing in notebooks is laggy #1977

Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
d042c3c
fix: Typing in notebooks is laggy
mofojed May 1, 2024
2c57223
Change to a throttled instead of debounced
mofojed May 1, 2024
c318d92
Add useThrottledCallback hook
mofojed May 2, 2024
af19103
Clean up useThrottledCallback
mofojed May 2, 2024
6e303eb
Flush the throttled callback
mofojed May 2, 2024
f7e61d6
Clean up useThrottledCallback and tests
mofojed May 2, 2024
dfd35ca
Clean up from review, update comments
mofojed May 2, 2024
1739edb
Address review comments
mofojed May 2, 2024
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
10 changes: 9 additions & 1 deletion packages/dashboard/src/DashboardLayout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import type {
import Log from '@deephaven/log';
import { usePrevious } from '@deephaven/react-hooks';
import { RootState } from '@deephaven/redux';
import throttle from 'lodash.throttle';
import { useDispatch, useSelector } from 'react-redux';
import PanelManager, { ClosedPanels } from './PanelManager';
import PanelErrorBoundary from './PanelErrorBoundary';
Expand Down Expand Up @@ -46,6 +47,8 @@ const DEFAULT_LAYOUT_CONFIG: DashboardLayoutConfig = [];

const DEFAULT_CALLBACK = (): void => undefined;

const STATE_CHANGE_DEBOUNCE_MS = 1000;
Comment thread
mofojed marked this conversation as resolved.
Outdated

// If a component isn't registered, just pass through the props so they are saved if a plugin is loaded later
const FALLBACK_CALLBACK = (props: unknown): unknown => props;

Expand Down Expand Up @@ -228,6 +231,11 @@ export function DashboardLayout({
}
}, [dehydrateComponent, isItemDragging, lastConfig, layout, onLayoutChange]);

const throttledHandleLayoutStateChanged = useMemo(
() => throttle(handleLayoutStateChanged, STATE_CHANGE_DEBOUNCE_MS),
Copy link
Copy Markdown
Collaborator

@mattrunyon mattrunyon May 1, 2024

Choose a reason for hiding this comment

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

This will need the trailing option I think so the final call is always run. Not sure if that's a default or how the specifics work, but make sure the final call is always executed.

Copy link
Copy Markdown
Member Author

@mofojed mofojed May 1, 2024

Choose a reason for hiding this comment

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

trailing=true is the default.

Copy link
Copy Markdown
Collaborator

@mattrunyon mattrunyon May 1, 2024

Choose a reason for hiding this comment

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

Looks like the default is leading=true and trailing=true. So I think it should be fine? But might want to add a flush call on unmount just in case there's a pending update

Copy link
Copy Markdown
Collaborator

@mattrunyon mattrunyon May 1, 2024

Choose a reason for hiding this comment

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

Yep so this will currently call immediately, then again at the end of the timeout if the throttled function has been called again. flush will call whatever the last invoked was ignoring timeout and will not trigger a call at the end unless it has been invoked again.

So flush on unmount to ensure there's no queued saves

[handleLayoutStateChanged]
);

const handleLayoutItemPickedUp = useCallback(
(component: Container) => {
const componentId = LayoutUtils.getIdFromContainer(component);
Expand Down Expand Up @@ -269,7 +277,7 @@ export function DashboardLayout({
setLayoutChildren(layout.getReactChildren());
}, [layout]);

useListener(layout, 'stateChanged', handleLayoutStateChanged);
useListener(layout, 'stateChanged', throttledHandleLayoutStateChanged);
useListener(layout, 'itemPickedUp', handleLayoutItemPickedUp);
useListener(layout, 'itemDropped', handleLayoutItemDropped);
useListener(layout, 'componentCreated', handleComponentCreated);
Expand Down
⚡