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

Fixed playground draggable block while scrolled #5844

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Legonois
Copy link

@Legonois Legonois commented Apr 8, 2024

Last pull request did not include the fix for the actual on drop event, only for the placement of the target line. This adjusts the drop event to account for scrolling

Adjusted the ondrop so the element is able to be dropped at the top while scrolled
Copy link

vercel bot commented Apr 8, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
lexical ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 25, 2024 8:59pm
lexical-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 25, 2024 8:59pm

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Apr 8, 2024
@ivailop7
Copy link
Collaborator

ivailop7 commented Apr 8, 2024

I guess this PR is meant to supersede the #5839 ? So 5839 is closable?

@Legonois
Copy link
Author

Legonois commented Apr 8, 2024

Correct, is there a way to append a PR without making a new one? I'm still new with open-source contributions still lol

@ivailop7
Copy link
Collaborator

ivailop7 commented Apr 8, 2024

Correct, is there a way to append a PR without making a new one? I'm still new with open-source contributions still lol

You should have just pushed to the original branch, it would have reflected in the existing Pull Request. No worries! We were all once beginners :) I'll close the other PR.

@ivailop7
Copy link
Collaborator

ivailop7 commented Apr 8, 2024

Still not able to reproduce this issue in the current Playground

@@ -361,7 +361,8 @@ function useDraggableBlockMenu(
return true;
}
const targetBlockElemTop = targetBlockElem.getBoundingClientRect().top;
if (pageY / calculateZoomLevel(target) >= targetBlockElemTop) {
const adjsutedTop = targetBlockElemTop + window.scrollY;
if (pageY / calculateZoomLevel(target) >= adjsutedTop) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: typo in adjsutedTop

Copy link
Collaborator

Choose a reason for hiding this comment

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

@Legonois are you planning on fixing the typo, so we can proceed forward with the merge

@Legonois
Copy link
Author

Just checked the deployments for the bug and it seems fixed!

Tell me if there is anything else I need to do before this can be pulled to main

@potatowagon
Copy link
Contributor

Just checked the deployments for the bug and it seems fixed!

Tell me if there is anything else I need to do before this can be pulled to main

perhaps rebase and fix the typo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants