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

Disable pointer events on unselected iframe #1028

Closed
wants to merge 1 commit into from
Closed

Conversation

ellatrix
Copy link
Member

@ellatrix ellatrix commented Jun 5, 2017

See #1002. Props @westonruter.

@ellatrix
Copy link
Member Author

ellatrix commented Jun 5, 2017

Note: the iframe-overlay class is not used anywhere atm.

@ellatrix ellatrix requested review from mtias and jasmussen June 6, 2017 11:10
@jasmussen
Copy link
Contributor

jasmussen commented Jun 6, 2017

Nice! Seems to work as intended. That is, when you click it the first time, it doesn't start, it selects the block. Nice. Clicking the second time, starts the video.

Are we sure we want the latter? Do we want to let you play the video in the editor at all? I have no strong opinion here, and I defer to those of you with good arguments on hand.

If we do want to allow it, though, I noticed that the invisible area to the left and right of the inline toolbar is unclickable no matter any preceeding clicks. This area:

screen shot 2017-06-06 at 13 15 25

Not a deal breaker, but if we do want to allow the video to play, we should consider making that area clickable also.

General 👍 though!

@ellatrix
Copy link
Member Author

ellatrix commented Jun 6, 2017

I'll look into the issue. Yeah, maybe let's see how this goes, and if we still want to block interacting with selected iframes at a later point, we can. :)

@ellatrix
Copy link
Member Author

ellatrix commented Jun 6, 2017

Okay, I have to admit that it would not be as straightforward as I thought. It looks like we're better off with some kind of wrapper component like <ClickToPlay message="..." icon="..." /> because embedding a tweet does not create an iframe. Or just a wrapper component to block interaction entirely for the time being.

@ellatrix
Copy link
Member Author

ellatrix commented Jun 6, 2017

I'll close this PR as the approach does not accomplish what was intended.

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 this pull request may close these issues.

2 participants