-
Notifications
You must be signed in to change notification settings - Fork 5
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
ModalCloseWatcher should possibly use a promise rather than close event #34
Comments
I'm torn on this largely because of the symmetry with Maybe we should have both?? |
Another issue is that the promise-for-one-time-event thing works best when it's OK to "subscribe" even after the event has been fired. E.g. I'm not sure the same thing holds for close signals. If you do |
Ah, that's reasonable.
I think, realistically, yes? It's going to depend on the situation, of course, but if you did some setup for the modal (some styling elsewhere on the page, for example) and need to do teardown after it closes, you'd want this to still happen even if you missed the opening. The opposite is true as well, of course, where you don't want to do something in response to an already-closed dialog. However, leaking an event listener because you registered it after missing the event isn't great (tho probably insignificant...) It looks like there's no state exposed for whether the CloseWatcher is still pending or already closed; exposing that would at least let code tell whether they should even register for the close event or not, which would satisfy my use-case. |
I'm still noodling on the rest of the discussion, but I want to agree with:
This seems like a good thing to add independent of the promise vs. event discussion. |
In https://github.com/slightlyoff/history_api/blob/master/history_and_modals.md the ModalCloseWatcher's primary interaction with the developer is firing either zero or one
close
events. It never fires multipleclose
events; once theclose
event has fired, the object doesn't do anything anymore; and theclose
event is purely informative, so nothing you do in response can affect the object.This is the exact interaction pattern that Promises are better suited for. Could the
close
event instead become a.closed
Promise attribute?(The
beforeclose
proposed event should stay as it is; it can fire multiple times, you canpreventDefault
it, etc.)The text was updated successfully, but these errors were encountered: