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

theDialogManager addHandler no arguments passed to handler function #2843

Open
4 tasks done
nelu opened this issue Jan 5, 2025 · 2 comments
Open
4 tasks done

theDialogManager addHandler no arguments passed to handler function #2843

nelu opened this issue Jan 5, 2025 · 2 comments

Comments

@nelu
Copy link
Contributor

nelu commented Jan 5, 2025

Please complete the following tasks.

  • Web browser cache cleared
  • Link provided to install script if applicable
  • Not using broken rtinst install script
  • Web browser, ruTorrent, PHP and OS version provided

Tell us about your environment

Docker

Tell us how you installed ruTorrent

Docker

Describe the bug

The handler and the previous queued function are being called with no arguments in addHandler method:

handler();

	addHandler: function(id, type, handler) {
		if ($type(this.items[id])) {
			const existing = this.items[id][type];
			if (existing) {
				this.items[id][type] = function() {
					existing();
					handler();
				};
			} else {
				this.items[id][type] = handler;
			}
		}
		return this;
	},

The proper function handler signature is this (when called):

this.items[id].afterHide(id);

			this.items[id].afterHide(id);

Steps to reproduce

Check if handler function has any arg value when multiple handler are registered for the same event.

Expected behavior

Use something like this

				this.items[id][type] = function(id) {
					existing(id);
					handler(id);
				};

Additional context

Noticed while working with the _getDir plugin on the hide events

@jevenski
Copy link
Contributor

jevenski commented Jan 6, 2025

This is not a very efficient nor maintainable solution to manage dialog open/close event handlers in today's perspective. I'm thinking about defining custom events for dialog opening and closing events, and register/deregister event handlers for them. This was inspired by Bootstrap's show, shown, hide, and hidden events. We could have migrated to Bootstrap's Modal component for easier management, but it has a limitation that only one dialog window is allowed to open at the same time. So we need to stick with our own dialog system and adopt those custom events.

@nelu
Copy link
Contributor Author

nelu commented Jan 6, 2025

You could use the standard event system available: element.on / trigger

       // deregistering when cleaning up
        var unbind = function (e, task) {
            if (task.no === runTask.no) {
                def.resolve(task);
            }
        };

        $(document).on(flm.EVENTS.taskDone, unbind);

        def.promise().then(function (task) {
            $(document).off(flm.EVENTS.taskDone, unbind);
            return task;
        });
....
        $(document).trigger(flm.EVENTS.taskDone, task);

Which would make sense since this ships natively with the browser/jq. see https://api.jquery.com/category/events/event-handler-attachment/ and https://developer.mozilla.org/en-US/docs/Web/Events/Creating_and_triggering_events
Also uing bootstraps dialog system ON TOP of the current system seems quite doable (except bootstrap js part).

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

No branches or pull requests

2 participants