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

Drawer still dimissible with snappoints #161

Closed
imclint21 opened this issue Nov 7, 2023 · 28 comments · Fixed by #179
Closed

Drawer still dimissible with snappoints #161

imclint21 opened this issue Nov 7, 2023 · 28 comments · Fixed by #179

Comments

@imclint21
Copy link

Hi,

I open this issue for centralise this problem:

After updating to the last version (0.7.8), and using snappoints, the drawer still dismissible, when it shouldn't.

Here is the code I'm using:

<Drawer.Root
	shouldScaleBackground={true}
	open={open}
	snapPoints={[snapPoint1 || "290px", snapPoint2 || "650px"]}
	activeSnapPoint={snap}
	setActiveSnapPoint={setSnap}
	dismissible={false}
	modal={false}>
</Drawer.Root>

Regards

@emilkowalski
Copy link
Owner

This has been fixed in #166

@imclint21
Copy link
Author

Just updated to the last version, and it does not seems fixed anyway =(

@emilkowalski
Copy link
Owner

Please provide a codesandbox demo in that case, I'm unable to reproduce.

@imclint21
Copy link
Author

@imclint21
Copy link
Author

imclint21 commented Nov 14, 2023

It might be a misunderstood of the vision that you have of the a non-dismissible drawer actually.

For me snapPoints and dismissible={false} should give a drawer that can only reach snap points, and cannot be closed at all without dragging the drawer to the bottom of the screen.

Drawer can be a required part of the UI, in my case, if the drawer is closed, the user cannot order nothing (the order summary is inside).

@imclint21
Copy link
Author

I just tried this, it works as a fallback, but I'd prefer a real non-dismissible drawer.

Don't work however with too small timeout, like 100ms.

useEffect(() => {
	if (!open) setTimeout(() => {
		setOpen(true);
		setSnap(snapPoint1 || "290px");
	}, 500);
}, [open]);

@IsaiahHarris
Copy link

IsaiahHarris commented Nov 19, 2023

Any updates on this? I have the same issue. Drawer that has snap points and dismissible set to false is still able to close.

@emilkowalski
Copy link
Owner

emilkowalski commented Nov 21, 2023

What's the problem with this one? Your code was incorrect, I edited a bunch of lines.

@IsaiahHarris
Copy link

@emilkowalski i have the same problem. Please provide example for drawer that cannot close but has snap points

@emilkowalski
Copy link
Owner

@IsaiahHarris Here you go mate - example

@IsaiahHarris
Copy link

@emilkowalski i'm still able to close the drawer with that demo. see video

Untitled.mov

@IsaiahHarris
Copy link

@emilkowalski is there a communication issue I am missing?

The issue is the drawer with snapPoints, and dismissible={false}, will still close.

The expected behavior is that the drawer should not close.

Currently there is no known way to have a drawer that stays on the page and is non dismissible that has snapPoints.

Please let me know if you need any more information about the issue if I can clear up any confusion.

cc: @imclint21 @xih

@imclint21
Copy link
Author

No sorry I'm sure @emilkowalski will find time to manage this issue, then I could release our customers dashboard :D

@emilkowalski
Copy link
Owner

@emilkowalski is there a communication issue I am missing?

The issue is the drawer with snapPoints, and dismissible={false}, will still close.

The expected behavior is that the drawer should not close.

Currently there is no known way to have a drawer that stays on the page and is non dismissible that has snapPoints.

Please let me know if you need any more information about the issue if I can clear up any confusion.

cc: @imclint21 @xih

No communication issues, I just can’t reproduce it, it doesn’t close for me 🤔 This is interesting, I’ll look into it as soon as I have some spare time. It’ll be most likely this weekend as I’m pretty busy. Sorry about that!

@IsaiahHarris
Copy link

@emilkowalski awesome, I am open and willing to hopping on a video call to help you reproduce and contribute. It’s pretty simple to reproduce. Please feel free to reach out at isaiah.harris.hi@gmail.com

@imclint21
Copy link
Author

@emilkowalski is there a communication issue I am missing?
The issue is the drawer with snapPoints, and dismissible={false}, will still close.
The expected behavior is that the drawer should not close.
Currently there is no known way to have a drawer that stays on the page and is non dismissible that has snapPoints.
Please let me know if you need any more information about the issue if I can clear up any confusion.
cc: @imclint21 @xih

No communication issues, I just can’t reproduce it, it doesn’t close for me 🤔 This is interesting, I’ll look into it as soon as I have some spare time. It’ll be most likely this weekend as I’m pretty busy. Sorry about that!

Which browser do you use btw?

@IsaiahHarris
Copy link

IsaiahHarris commented Nov 22, 2023

@emilkowalski is there a communication issue I am missing?
The issue is the drawer with snapPoints, and dismissible={false}, will still close.
The expected behavior is that the drawer should not close.
Currently there is no known way to have a drawer that stays on the page and is non dismissible that has snapPoints.
Please let me know if you need any more information about the issue if I can clear up any confusion.
cc: @imclint21 @xih

No communication issues, I just can’t reproduce it, it doesn’t close for me 🤔 This is interesting, I’ll look into it as soon as I have some spare time. It’ll be most likely this weekend as I’m pretty busy. Sorry about that!

Which browser do you use btw?

@imclint21 I am able to reproduce with 100% consistency on chrome and firefox

@imclint21
Copy link
Author

me too, but I was asking to @emilkowalski :)

@IsaiahHarris
Copy link

me too, but I was asking to @emilkowalski :)
@imclint21 oh got it haha

@IsaiahHarris
Copy link

Any updated on this? Please let me know if you need help reproducing @emilkowalski

@rortan134
Copy link
Contributor

Same problem here

@rortan134
Copy link
Contributor

rortan134 commented Dec 2, 2023

Dirty workaround while this isn't fixed

Use forceMount on the content and override the translate3d style used to move the drawer out of the screen with the exposed --snap-point-height variable

"use client";

import * as React from "react";
import { Drawer } from "vaul";

const snapPoints = ["148px", "355px", 1];
// Taken from https://github.com/emilkowalski/vaul/blob/main/src/constants.ts
const VELOCITY_THRESHOLD = 0.4;

export function MyDrawer() {
  const [activeSnapPoint, setActiveSnapPoint] = React.useState<
    number | string | null
  >("148px");
  const drawerSheetRef = React.useRef<HTMLDivElement>(null);
  const previousDragYRef = React.useRef(0);
  const dragStartTimeRef = React.useRef<Date>();
  const minSnapPointHeight = drawerSheetRef.current
    ? getComputedStyle(drawerSheetRef.current).getPropertyValue(
        "--snap-point-height",
      )
    : "0px";

  function preventDrawerClose() {
    setActiveSnapPoint(snapPoints[0]);
    drawerSheetRef.current?.style.setProperty(
      "transform",
      `translate3d(0, ${minSnapPointHeight}, 0)`,
    );
  }

  function checkDrawerBoundaries() {
    if (
      drawerSheetRef.current &&
      drawerSheetRef.current.getBoundingClientRect().top >
        parseInt(minSnapPointHeight)
    ) {
      preventDrawerClose();
    }
  }

  // Stops flicks from closing the drawer
  function handleOnPointerUp(event: React.PointerEvent) {
    const dragEndTime = new Date().getTime();
    const dragStartTime = dragStartTimeRef.current
      ? dragStartTimeRef.current.getTime()
      : 0;
    const timeTaken = dragEndTime - dragStartTime;
    const distanceMoved = previousDragYRef.current - event.screenY;
    const dragVelocity = Math.abs(distanceMoved) / timeTaken;
    const hasVelocityMetThreshold = dragVelocity > VELOCITY_THRESHOLD;
    const hasDraggedUp = distanceMoved > 0;

    if (hasVelocityMetThreshold && !hasDraggedUp) {
      // setTimeout makes sure updates are ran after vaul's
      setTimeout(() => {
        preventDrawerClose();
      }, 10);
    }
  }

  return (
    <Drawer.Root
      open
      dismissible={false}
      modal={false}
      snapPoints={snapPoints}
      activeSnapPoint={activeSnapPoint}
      setActiveSnapPoint={setActiveSnapPoint}
      onDrag={checkDrawerBoundaries}
    >
      <Drawer.Portal forceMount>
        <Drawer.Content
          ref={drawerSheetRef}
          asChild // asChild is needed in order to merge onPointer events without overriding vaul's
        >
          <div
            data-state="open" // always open
            vaul-drawer-visible="true"
            className="flex flex-col max-w-md mx-auto w-full p-4 pt-5"
            onPointerDown={(event) => {
              previousDragYRef.current = event.screenY;
              dragStartTimeRef.current = new Date();
            }}
            onPointerUp={handleOnPointerUp}
          >
            {/* ... */}
          </div>
        </Drawer.Content>
      </Drawer.Portal>
    </Drawer.Root>
  );
}

Full example here

@imclint21
Copy link
Author

ah nice!

@IsaiahHarris
Copy link

@emilkowalski first time working with this repo, how often do releases come out?

@IsaiahHarris
Copy link

@emilkowalski first time working with this repo, how often do releases come out?

@imclint21 do you have any experiences with releases?

@imclint21
Copy link
Author

imclint21 commented Dec 8, 2023

It's not released atm @IsaiahHarris.
image

Would be nice @emilkowalski to have a github action for autorelease.

@IsaiahHarris
Copy link

@emilkowalski any idea when this will get released?

@emilkowalski
Copy link
Owner

@IsaiahHarris This is released now!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants