cloudflare / embed-box

Universal install guide for embed codes and CMS plugins.
http://embedbox.io
MIT License
77 stars 12 forks source link

Hitting the modal header back too quickly breaks navigation #82

Closed adamschwartz closed 8 years ago

adamschwartz commented 8 years ago

If you hit back just after a navigation into the instructions, you won’t be able to click any of the entries in the target search list. @TeffenEllis I can make a repro video if necessary. Just lmk.

GirlBossRush commented 8 years ago

Disregard, found the bug

~~I'm unable to reproduce this behavior on Safari, mobile Safari, Chrome, or Firefox. This seems to be consistent whether EmbedBox is in a modal or inline, with keyboard or mouse.~~

We have logic that prevents keypresses from being activated while a transition but it isn't tied to the target entry event handlers.

https://github.com/EagerIO/EmbedBox/blob/master/app/components/application/index.js#L84-L119

Perhaps this is a browser repaint bug in transitions? Maybe the instruction screen is still in the DOM but the target search is painted?

Can you provide video with instructions that reproduce the bug? If this ends up being a show stopper I'll move this into a version 1.0 patch

Thanks! 🐳

GirlBossRush commented 8 years ago

I was able to reproduce this bug, but only sporadically when my machine lagged. It seems that Chrome's transition event is throttled. I have a feeling that there's a combination of DOM repaints and element changes that convince the browser to drop transitions.

Commit 7110bd62cbb39fa723c3495dbd1f4422b711d7a0 reduces the chance of this scenario occurring and forces the transitionend handler after a timeout. This will prevent the UI from ever locking up if Chrome drops the transition.

I'm going to mark this as closed for the 1.0 release. Please reopen it if there is a show stopper bug.

Cheers 🐳

adamschwartz commented 8 years ago

I’m still seeing this issue, but with https://github.com/EagerIO/EmbedBox/commit/7110bd62cbb39fa723c3495dbd1f4422b711d7a0 it is presenting slightly differently. Rather than completely disabling further navigation, what instead happens is that navigation is temporarily blocked while the transitioning is still happening.

Since when hitting the back button sometimes it still skips the transition (one of the remaining underlying issues), this means that for the duration of what transition should have occurred, the cursor is default not pointer and clicking the target search list entries does nothing.

adamschwartz commented 8 years ago

Fixed with https://github.com/EagerIO/EmbedBox/commit/62ac759d374229b76b6acb5ba02212be09d4237c