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

[Super WIP] iFrame/ShadowDOM support #202

Closed
wants to merge 2 commits into from
Closed

[Super WIP] iFrame/ShadowDOM support #202

wants to merge 2 commits into from

Conversation

tsov
Copy link
Member

@tsov tsov commented Apr 5, 2018

WIP

iframe-dragging-2

@tsov tsov changed the base branch from master to v1.0.0-beta.7 April 5, 2018 00:09
@kachurun
Copy link

kachurun commented Apr 7, 2018

Its amazing, man!

@ncovercash
Copy link

ncovercash commented Apr 7, 2018

Interesting. I thought iframes were basically immutable for security reasons? Or maybe that just applies to cross-domain stuff.

@tsov
Copy link
Member Author

tsov commented Apr 9, 2018

I thought iframes were basically immutable for security reasons?

@smileytechguy This can be the case when the iframe contents are on a different origin (cross-origin). Access to the iframes document will be a requirement for using draggable with iframes.

I'll make sure to add that to the docs 👍

@ncovercash
Copy link

Alright, wasn't sure. This is super cool! 👍

@@ -1,15 +0,0 @@
# editorconfig.org
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we deleting this because we don't find any value in it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Was just temporary, because it was messing up some formatting for me

@beefchimi
Copy link
Contributor

@tsov when your ready to return to this PR, lets chat so I can better understand what you want to achieve with the Example - I can then take over the example portion of this PR.

@beefchimi beefchimi added feature-request requesting new functionality to be added examples specific to the examples directory labels Apr 21, 2018
@tsov
Copy link
Member Author

tsov commented Apr 23, 2018

@beefchimi absolutely! As you may have seen, I need to create a new layout to serve through an iframe, but we can chat IRL about it.

Would love to finish this up soon

@beefchimi
Copy link
Contributor

I don't know why it says I closed this... I definitely didn't

@deiucanta
Copy link

deiucanta commented May 2, 2018

@tsov any idea if it's possible to do the drag the other way around? (from inside to outsite)

The event handler on the main host is not triggered on mousemove. Instead it get triggered on the iframe host with negative values on clientX and clientY.

Calling preventDefault() on mousedown makes it work but only in Chrome, not in Firefox.


if (this.startedInsideIframe && (clientX < 0 || clientY < 0)) {
const iframeElements = [...document.querySelectorAll('iframe')];
const iframeElement = iframeElements.find((iframe) => iframe.contentDocument === currentHost);

Choose a reason for hiding this comment

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

what about using event.view.frameElement?


let target;

if (this.startedInsideIframe && (clientX < 0 || clientY < 0)) {

Choose a reason for hiding this comment

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

clientX might be outside the iframe on the right side. You can check the upper limit using iframe's width or using elementFromPoint

const frameRect = frameElement.getBoundingClientRect()
const globalX = clientX + frameRect.left
const globalY = clientY + frameRect.top
const elementFromPoint = document.elementFromPoint(globalX, globalY)

return elementFromPoint === frameElement

This will work even if there are some elements displayed over the iframe element.

@deiucanta
Copy link

I figured it out. You are using elementFromPoint and I was using simply event.target.

@petersooley
Copy link

Why is this closed?

@tsov
Copy link
Member Author

tsov commented Jun 20, 2018

I believe it was an accident. We are still planning to add ShadowDOM and iFrame support, but are currently focusing on getting keyboard support for draggable out. That way we can make sure this code works for all sensors that draggable provides (touch, mouse, keyboard, force touch)

Stay tuned

@StephenEsser
Copy link

Hi, @tsov. Are there any updates on dragging between iFrames?

@StephenEsser
Copy link

Just checking in again, is iFrame support still planned? Are there any updates on this?

cc: @beefchimi @tsov

@beefchimi
Copy link
Contributor

@StephenEsser @tsov probably won't reply - I would say that iFrame support is not going to be happening 😢

@owen-m1
Copy link
Contributor

owen-m1 commented Jun 16, 2019

Dang, I went ahead and started implementing this and I had no idea this PR even existed. Anyways, the PR I started working on #348 implements ShadowDOM support without a hosts option, it just works around shadowDOMs as if they were part of the regular DOM. This is what SortableJS does as was mentioned here.

I personally think it makes more sense to just have it work around the shadowDOMs rather than having the user specify them all... like in the scenario where the containers are shadowDOM components and are added dynamically, or nested draggable components, etc.

I haven't looked into Iframe support yet.

@mqliutie
Copy link

Hi, The gif shows I didn't find in the example page

@TimeBandit
Copy link

TimeBandit commented Aug 14, 2019

I believe this is affecting my end-to-end. I am running tests inside Cypress.js which uses iframes to show your tests running. We are using draggable to simulate a drag and drop. This error message is seen when the mouseup event is triggered.

Uncaught TypeError: Failed to execute 'elementFromPoint' on 'Document': The provided double value is non-finite.

Please comment or provide a workaround. I think the issue is line 173

I should add I am running Ubuntu.

Update: 15 Aug 2019
I have a workaround for this, to anyone developing your tests, this is a browser issue. From the test runner select Electron 61 as your test browser.

@tjmonsi
Copy link

tjmonsi commented Apr 21, 2020

Will this ever be part of draggable?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
examples specific to the examples directory feature-request requesting new functionality to be added
Projects
None yet
Development

Successfully merging this pull request may close these issues.