Skip to content

Commit

Permalink
Fix slider number control performance (#6374)
Browse files Browse the repository at this point in the history
**Problem:**
When using the slider in the SliderNumber controls (such as the
PromiseCard's BorderRadius in my screenshot) in the component inspector
section, the displayed number updates unbearably slowly.
<img width="653" alt="image"
src="https://github.com/user-attachments/assets/159bac67-2bef-4e4f-bf5e-d5fcfc704687">

The reason is that the slider and number are actually two separate
controls, and the number control only gets its state updated in low
priority mode once the interaction is finished.

**Fix:**
Use a shared local state for the value we immediately set while using
the slider.

**Commit Details:**
- Use `usePropControlledStateV2` as shared local state for the transient
updates

**Bonus mini fixes:**
- wrap RemixNavigationBar in LowPriorityState 
- speeding up Tooltip / tippy

**Manual Tests:**
I hereby swear that:

- [x] I opened a hydrogen project and it loaded
- [x] I could navigate to various routes in Preview mode
  • Loading branch information
balazsbajorics authored Sep 16, 2024
1 parent c7d1524 commit 043570d
Show file tree
Hide file tree
Showing 4 changed files with 154 additions and 144 deletions.
5 changes: 4 additions & 1 deletion editor/src/components/editor/canvas-toolbar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ import {
keyToString,
shortcutDetailsWithDefaults,
} from './shortcut-definitions'
import { LowPriorityStoreProvider } from './store/store-context-providers'

export const InsertMenuButtonTestId = 'insert-menu-button'
export const InsertOrEditTextButtonTestId = 'insert-or-edit-text-button'
Expand Down Expand Up @@ -579,7 +580,9 @@ export const CanvasToolbar = React.memo(() => {
)
: null}
{/* Live Mode */}
{showRemixNavBar ? wrapInSubmenu(<RemixNavigationBar />) : null}
<LowPriorityStoreProvider>
{showRemixNavBar ? wrapInSubmenu(<RemixNavigationBar />) : null}
</LowPriorityStoreProvider>
</FlexColumn>
)
})
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import fastDeepEquals from 'fast-deep-equal'
import React from 'react'
import ReactDOM from 'react-dom'
import { MetadataUtils } from '../../../../core/model/element-metadata-utils'
import { isRight } from '../../../../core/shared/either'
import { isJSXElement } from '../../../../core/shared/element-template'
Expand Down Expand Up @@ -69,6 +70,7 @@ import type { DEPRECATEDSliderControlOptions } from '../../controls/slider-contr
import { StringControl } from '../../controls/string-control'
import { UIGridRow } from '../../widgets/ui-grid-row'
import { HtmlPreview, ImagePreview } from './property-content-preview'
import { usePropControlledStateV2 } from '../../common/inspector-utils'

export interface ControlForPropProps<T extends RegularControlDescription> {
propPath: PropertyPath
Expand Down Expand Up @@ -492,20 +494,37 @@ const NumberWithSliderControl = React.memo(
},
) => {
const { propName, propMetadata, controlDescription } = props
const { onTransientSubmitValue, onSubmitValue } = propMetadata

const controlId = `${propName}-slider-property-control`
const value = getValueOrUndefinedFromPropMetadata(propMetadata) ?? 0
const valueComingFromProps = getValueOrUndefinedFromPropMetadata(propMetadata) ?? 0

const [valueToShow, setValueToShow] = usePropControlledStateV2(valueComingFromProps)

const onChangeValue = React.useCallback(
(value: any, transient: boolean = false) => {
ReactDOM.flushSync(() => {
setValueToShow(value)
})
if (transient) {
onTransientSubmitValue(value)
} else {
onSubmitValue(value, transient)
}
},
[setValueToShow, onTransientSubmitValue, onSubmitValue],
)

return (
<UIGridRow padded={false} variant={'<--------auto-------->|--45px--|'}>
<SliderControl
key={`${controlId}-slider`}
id={`${controlId}-slider`}
testId={`${controlId}-slider`}
value={value}
onTransientSubmitValue={propMetadata.onTransientSubmitValue}
onForcedSubmitValue={propMetadata.onSubmitValue}
onSubmitValue={propMetadata.onSubmitValue}
value={valueToShow}
onTransientSubmitValue={onChangeValue}
onForcedSubmitValue={onChangeValue}
onSubmitValue={onChangeValue}
controlStatus={propMetadata.controlStatus}
controlStyles={propMetadata.controlStyles}
DEPRECATED_controlOptions={props.controlOptions}
Expand All @@ -514,7 +533,7 @@ const NumberWithSliderControl = React.memo(
id={`${controlId}-number`}
testId={`${controlId}-number`}
key={`${controlId}-number`}
value={value}
value={valueToShow}
onTransientSubmitValue={propMetadata.onTransientSubmitValue}
onForcedSubmitValue={propMetadata.onSubmitValue}
onSubmitValue={propMetadata.onSubmitValue}
Expand Down
Loading

0 comments on commit 043570d

Please sign in to comment.