Prototype a version relying on ‘CloseWatcher’

[KittyGiraudel]

No description provided.

[KittyGiraudel]

First thing I don’t love about the CloseWatcher API is that it’s single-use.

Once the watcher received a close event, it no longer processes anything else. That means we need to instantiate a new watcher every time we open the dialog.

There is also no way to know from an existing instance whether or not it’s been consumed or if it can process a close event. That’s not exposed.

[mayank99]

Yeah I agree the API shape is a bit weird. I guess it makes sense that you don't want to constantly keep listening for a close request.

Does this cause an actual problem?

[KittyGiraudel]

Second thing I find confusing is the difference between .close(..) and .requestClose(..). Honestly not sure I get it.

[mayank99]

From what I understand, the difference is that requestClose() allows the close event to be cancelled (using preventDefault) whereas close() will force close it.

For the main purpose of CloseWatcher (user-initiated close requests), you might be able to get away without using either of these methods.

Edit: It looks like 290eb14 removes the requestClose 👍

[KittyGiraudel]

I don’t know whether we should remove the watcher at that point, like this:

Suggested change

	closeWatcher.onclose = event => {
	this.hide(event)
	}
	closeWatcher.onclose = event => {
	this.hide(event)
	closeWatcher = null
	}

[mayank99]

Maybe it should be defined within local scope to ensure that it automatically gets garbage collected?

Edit: It looks like 290eb14 changes the approach to use a locally scoped CloseWatcher 👍

[KittyGiraudel]

This is a breaking change, because we no longer can provide access to the “closer” (as in the element that was interacted with to close the dialog) via the close watcher. That’s because we can’t pass an event to the .close(..) or .requestClose(..) method which would be forwarded to the onclose callback. So this is a bit shitty and may be a deal breaker for this integration.