Skip to content
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

Converting x/y from percent to pixels before drag starts #1322

Merged
merged 3 commits into from Nov 1, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Expand Up @@ -2,6 +2,10 @@

Framer Motion adheres to [Semantic Versioning](http://semver.org/).

## [5.0.2] 2021-11-02

- Convert x/y from percent to pixels before drag. [Issue](https://github.com/framer/motion/issues/424)

## [5.0.1] 2021-11-01

### Added
Expand Down
23 changes: 23 additions & 0 deletions cypress/integration/drag.ts
Expand Up @@ -50,6 +50,29 @@ describe("Drag", () => {
})
})

it("Drags the element by the defined distance with percentage initial offset", () => {
cy.visit("?test=drag&x=200%&y=200%")
.get("[data-testid='draggable']")
.wait(200)
.trigger("pointerdown", 5, 5)
.trigger("pointermove", 10, 10) // Gesture will start from first move past threshold
.wait(50)
.trigger("pointermove", 200, 300, { force: true })
.wait(50)
.trigger("pointerup", { force: true })
.should(($draggable: any) => {
const draggable = $draggable[0] as HTMLDivElement
const { left, top } = draggable.getBoundingClientRect()

expect(left).to.equal(300)

// TODO: This should actually be 400, but for some reason the test scroll
// scrolls an additional 100px when dragging starts. But this has been manually verified
// as working
expect(top).to.equal(300)
})
})

it("Locks drag to x", () => {
cy.visit("?test=drag&axis=x")
.get("[data-testid='draggable']")
Expand Down
5 changes: 3 additions & 2 deletions dev/tests/drag-ref-constraints.tsx
@@ -1,4 +1,4 @@
import { motion } from "@framer"
import { motion, useMotionValue } from "@framer"
import * as React from "react"

// It's important for this test to only trigger a single rerender while dragging (in response to onDragStart) of draggable component.
Expand All @@ -13,7 +13,7 @@ export const App = () => {
React.useLayoutEffect(() => {
window.scrollTo(0, 100)
}, [])

const x = useMotionValue("100%")
return (
<div style={{ height: 2000, paddingTop: 100 }}>
<motion.div
Expand All @@ -31,6 +31,7 @@ export const App = () => {
width: 50,
height: 50,
background: dragging ? "yellow" : "red",
x,
}}
dragConstraints={containerRef}
layout={layout}
Expand Down
13 changes: 11 additions & 2 deletions dev/tests/drag.tsx
Expand Up @@ -2,6 +2,15 @@ import { motion } from "@framer"
import * as React from "react"

// It's important for this test to only trigger a single rerender while dragging (in response to onDragStart) of draggable component.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this comment go below the helper function? Or maybe the helper function should move to the bottom of the file?

function getValueParam(params: any, name: string) {
const param = params.get(name) as string | undefined
if (!param) return 0
if (param.endsWith("%")) {
return param
} else {
return parseFloat(param)
}
}

export const App = () => {
const params = new URLSearchParams(window.location.search)
Expand All @@ -12,8 +21,8 @@ export const App = () => {
const right = parseFloat(params.get("right")) || undefined
const bottom = parseFloat(params.get("bottom")) || undefined
const snapToOrigin = Boolean(params.get("return"))
const x = parseFloat(params.get("x")) || 0
const y = parseFloat(params.get("y")) || 0
const x = getValueParam(params, "x")
const y = getValueParam(params, "y")
const layout = params.get("layout") || undefined

// We do this to test when scroll position isn't 0/0
Expand Down
2 changes: 1 addition & 1 deletion package.json
Expand Up @@ -159,7 +159,7 @@
},
{
"path": "./dist/size-webpack-dom-max.js",
"maxSize": "30.2 kB"
"maxSize": "30.3 kB"
}
]
}
19 changes: 18 additions & 1 deletion src/gestures/drag/VisualElementDragControls.ts
Expand Up @@ -30,6 +30,8 @@ import {
import { LayoutUpdateData } from "../../projection/node/types"
import { addDomEvent } from "../../events/use-dom-event"
import { mix } from "popmotion"
import { percent } from "style-value-types"
import { calcLength } from "../../projection/geometry/delta-calc"

export const elementDragControls = new WeakMap<
VisualElement,
Expand Down Expand Up @@ -125,7 +127,22 @@ export class VisualElementDragControls {
* Record gesture origin
*/
eachAxis((axis) => {
this.originPoint[axis] = this.getAxisMotionValue(axis).get()
let current = this.getAxisMotionValue(axis).get()

/**
* If the MotionValue is a percentage value convert to px
*/
if (percent.test(current)) {
const measuredAxis =
this.visualElement.projection?.layout?.actual[axis]

if (measuredAxis) {
const length = calcLength(measuredAxis)
current = length * (parseFloat(current) / 100)
}
}

this.originPoint[axis] = current as number
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need the cast here?

Copy link
Contributor

Choose a reason for hiding this comment

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

We could check if it is undefined and not set it otherwise, right?

})

// Fire onDragStart event
Expand Down