Skip to content

Conversation

@WardCunningham
Copy link
Member

We also deprecate callback on emit which exists only in the Method plugin which Ward will upgrade.

@WardCunningham WardCunningham requested a review from dobbs September 6, 2025 17:55
@WardCunningham
Copy link
Member Author

Any other plugin expecting callbacks will be warned.

image

I'm unsure how Method synchronized. We did test that the bottles of beer computation still works.
http://start.fed.wiki/seventh-floor.html

script.emit(div, item)
done()
}
$('.retry').on('click', async function () {
Copy link
Member

@dobbs dobbs Sep 7, 2025

Choose a reason for hiding this comment

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

This is maybe not that important, but I think both the close and retry handlers would be improved by providing the dialog element as context like so:

$('.close', dialog).on('click', ...)
$('retry', dialog).on('click', ...) 

I think this would only matter if there were multiple errors showing in a lineup, which is probably uncommon.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm unable to find any jQuery documentation that explains what this second parameter is or means. This is as close as I come but this isn't what is being suggested here, I think.

https://api.jquery.com/context/

How would this second parameter be used? Are there further modifications required to make it work?

Thanks. Sorry to be so slow responding.

Copy link
Member

Choose a reason for hiding this comment

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

$('.close').on('click', ...) adds a click handler that applies to every element in the entire document with the close class. By specifying the second parameter it reduces the scope from document to just the specific dialog element.

Copy link
Member

Choose a reason for hiding this comment

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

Because we don't have many place in the code where retry or close classes are added, this is a pretty minor issue, I think.

@WardCunningham WardCunningham merged commit 983f71c into main Oct 4, 2025
3 checks passed
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.

3 participants