PIPRES-580 Improve payment methods list auto-scroll while dragging - #1489
Open
TLabutis wants to merge 1 commit into
Open
PIPRES-580 Improve payment methods list auto-scroll while dragging#1489TLabutis wants to merge 1 commit into
TLabutis wants to merge 1 commit into
Conversation
The payment methods list only scrolled near the bottom edge while dragging, and silently threw away the new order when the mouse was released while the page was still moving. Both came from the drag handlers living on the list container: .page-head is position:fixed over y=40-180 at every scroll offset, so a cursor in the top scroll zone is over the header and never over the container, and the ~37 dragleave events raised while crossing cards kept clearing the scroll interval, so identical holds scrolled -608, -864, -512px. Move dragover and drop to the document, replace setInterval/scrollBy(32) with a requestAnimationFrame loop that reads a pointer ref and ramps 4 to 22px per frame by edge proximity, and resolve the highlighted slot from the pointer against row edges cached in document coordinates at dragstart (measureRows), clamping past either end to the first or last slot. Rows do not reflow mid-drag, and re-reading 29 rects every frame right after the frame's scrollTop write forced a sync layout: identical holds spread 221px with the per-frame reads and 17px with the cache. The card onDragLeave prop is gone so the highlight survives the pointer passing behind the header. A release during auto-scroll produces no drop event at all, and a document-level preventDefault does not bring it back, so handleDragEnd replays the reorder when auto-scroll ran in the last 200ms, deferred 80ms to let the Escape keyup land and cancel it instead. keydown is not delivered during a native drag, keyup is, a few ms after dragend. No PHP, schema, config or translation change, so no upgrade script is needed, and the bundle URL is already cache busted with ?v=time().
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Dragging a card in Mollie > Payment methods had three problems, all measured on ps823 (PS 8.2.3, Mollie 6.4.5, 29 enabled methods, 3144px page in a 900px window):
.page-headisposition: fixedover y=40-180 at every scroll offset, so a cursor in the top scroll zone sits over the back office header, never over the list container, and the container'sdragovernever ran. The usable band was only y≈180-309.scrollBy(-32)every 16ms, and the ~37dragleaveevents raised while crossing five or six cards each cleared the interval. Identical holds gave -608, -864 and -512px.dropevent, noupdateMethodsOrderrequest, no message to the user. Isolated to the page moving, not to the cursor position: pinned at the end of the list a drop fires fine.How
dragoveranddropmoved from the list container to the document, so the pointer is tracked even while it is behind the fixed header.setInterval+scrollBy(32)replaced by arequestAnimationFrameloop reading apointerYref, 140px margin, speed ramped 4 to 22px per frame by edge proximity.measureRows), not from the cards' owndragoverevents, which fire far too rarely to keep up with an auto-scrolling list. Re-reading 29 rects per frame right after the frame'sscrollTopwrite forced a sync layout and cost frames: identical holds spread 221px that way, 17px with the cache. Rows do not reflow mid-drag.dragend, gated on "auto-scroll ran in the last 200ms" and deferred 80ms.keydownis not delivered to the page during a native drag butkeyupis, a few ms afterdragend, which is what lets a real Escape cancel be told apart from a swallowed drop.dragend'sdropEffectis"none"for both, so it cannot discriminate.onDragLeaveprop was removed so the highlight survives the pointer passing behind the header. The dragleave storm disappears with the container handlers, so the scroll no longer stutters.Upgrade safety
No PHP, schema, config or translation change, so no
Upgrade-6.4.6.phpis needed.ps_mol_payment_method.positionkeeps the same meaning and values, so existing merchant ordering is untouched. The bundle URL is already cache busted with?v=time()inAdminMolliePaymentMethodsController::init(), so no stale JS after upgrade and nothing to purge.dist/is gitignored andrelease.ymlbuilds it, so the zip ships the new code.Tested
13 scripted checks green on ps823: top edge at y=40 scrolls 3144 to 2022 over the fixed header; ramp gives +195 / +507 / +1116px at y=770/830/890; identical holds spread 17px; mid-scroll release lands on the highlighted slot with exactly one
updateMethodsOrderrequest; Escape mid-scroll saves nothing; Escape cancel, drop-on-self no-op, quiet-band drop and no post-drag scroll leak all behave; edge clamps verified in both directions with the highlight on slot 1 and slot 29; highlight matches the cursor with 0 mismatches.Front office reflection verified end to end: 19 Mollie options rendered in strictly ascending
positionorder, MobilePay dragged 28 to 1 moved it 19th to 1st in checkout, dragging it back restored it. Positions restored to baseline afterwards.Safari confirmed manually.
For QA
.nav ul/.nav.nav-pills/.nav.nav-tabs/ul.navfallback chain, so risk is low but the list itself has not been run there.views/js/admin/library/distwas not rebuilt (make build-react). It is gitignored, and the URL is cache busted per request, so it is never a cache issue.locator.dragTo()does nothing on this list, it sends too few moves to pass Chromium's native drag threshold. Scripting this needsmouse.downplus jitteredmouse.move, otherwise it reports a false failure.Changelog entry added under 6.4.6, since
release-6.4.5is already branched.