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.

Sign up for GitHub

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

Jump to bottom

Fix position of drag image #595

Merged
merged 2 commits into from
Jun 21, 2023
Merged

Fix position of drag image #595

merged 2 commits into from
Jun 21, 2023

Conversation

MetRonnie
Copy link
Contributor

Closes #593

I tried using transform-origin but that didn't work.

Note I've only tested this in my web app using Lumino but not a minimal example.

@welcome
Copy link

welcome bot commented Jun 13, 2023

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please make sure you followed the pull request template, as this will help us review your contribution more quickly.
welcome
You can meet the other Jovyans by joining our Discourse forum. There is also a intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

MetRonnie
MetRonnie commented Jun 13, 2023
View reviewed changes
packages/default-theme/style/tabbar.css
@@ -93,5 +93,4 @@
min-width: 125px;
border: none;
box-shadow: 1px 1px 2px rgba(0, 0, 0, 0.3);
transform: translateX(-40%) translateY(-58%);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed this because it's always overridden by

lumino/packages/dragdrop/src/index.ts

Lines 229 to 234 in 3f0be5c

protected moveDragImage(clientX: number, clientY: number): void {
if (!this.dragImage) {
return;
}
let style = this.dragImage.style;
style.transform = `translate(${clientX}px, ${clientY}px)`;

@MetRonnie MetRonnie mentioned this pull request Jun 13, 2023
Closed
@fcollonval fcollonval added the bug Something isn't working label Jun 20, 2023
@fcollonval
Copy link
Member

Thanks @MetRonnie

I took the liberty to push a commit on top of your PR to set the exact offset position of the cursor. What do you think?

@krassowski do you have time to review this one as it may impact CSS styling performance?

@krassowski
Copy link
Member

I think the proposed solution would be fine performance-wise as it only modifies position once when creating the drag node. I would recommend merging and once a new release is out I can run benchmarks to ensure there is no regressions and investigate manually if needed.

@MetRonnie
Copy link
Contributor Author

@fcollonval Nice 👍

@fcollonval
Copy link
Member

Thanks both - so I'm merging as is.

@fcollonval fcollonval merged commit f35a206 into jupyterlab:main Jun 21, 2023
@welcome
Copy link

welcome bot commented Jun 21, 2023

Congrats on your first merged pull request in this project! 🎉
congrats
Thank you for contributing, we are very proud of you! ❤️

@MetRonnie MetRonnie deleted the fix-drag-image branch June 21, 2023 08:51
@fcollonval fcollonval mentioned this pull request Jun 27, 2023
Open
gabalafou pushed a commit to gabalafou/lumino that referenced this pull request Jul 7, 2023
@MetRonnie @fcollonval @gabalafou
Fix position of drag image (jupyterlab#595)
78ce25d 
* Fix position of drag image

* Set exact offset between tab and cursor

---------

Co-authored-by: Frédéric Collonval <fcollonval@gmail.com>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees

@MetRonnie MetRonnie

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Drag image is rendered off-screen
3 participants
@MetRonnie @fcollonval @krassowski