Update page layoutdrag #7248
Update page layoutdrag #7248
Conversation
|
github-actions
bot
added
the
integration-tests: recommended
|
👋 Hi, this pull request contains changes to the source code that github/github-ui depends on. If you are GitHub staff, test these changes with github/github-ui using the integration workflow. Or, apply the |
| React.useEffect(() => { | ||
| stableOnDrag.current = onDrag | ||
| }, [onDrag]) | ||
| }) | ||
|
|
||
| React.useEffect(() => { | ||
| stableOnDragEnd.current = onDragEnd | ||
| }, [onDragEnd]) |
There was a problem hiding this comment.
assignment is cheaper than comparison for these
| } | ||
|
|
||
| /** | ||
| * Batches numeric updates so that only a single callback runs per animation frame. |
| * Batches numeric updates so that only a single callback runs per animation frame. | ||
| * Falls back to synchronous updates when the DOM is not available (SSR/tests). | ||
| */ | ||
| function useRafAccumulator(applyDelta: (delta: number) => void): RafAccumulatorControls { |
There was a problem hiding this comment.
is this something we could extract from this component and maybe use as a pattern in other cases similar to this one?
There was a problem hiding this comment.
yep - leaving here to minimize the files I needed to copy for now!
| stableOnDragEnd.current?.() | ||
| event.preventDefault() | ||
| } | ||
| // TODO: Support touch events |
There was a problem hiding this comment.
is this todo still relevant?
There was a problem hiding this comment.
yeah - I think it will need to be moved though!
|
|
||
| if (persist) { | ||
| try { | ||
| localStorage.setItem(widthStorageKey, resolvedWidth.toString()) |
There was a problem hiding this comment.
nit: could we look into IndexedDB here? I recall @JasonMore saying that it would be faster :)
|
@copilot can we further improve performance here by using element.SetPoinyerCapture /release pointer on start/stop for dragging by mouse. If we do this we can probably avoid a user-select none override on. Body * and just handle that here since the drag element will own the pointer, right? Can we also set a transform instead of width updates while dragging and only commit the width at the end of the drag to avoid document reflowing |
|
@mattcosta7 I've opened a new pull request, #7249, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
moving this PR to #7251 |
Closes #
Changelog
New
Changed
Removed
Rollout strategy
Testing & Reviewing
Merge checklist