Fomantic-UI icon indicating copy to clipboard operation
Fomantic-UI copied to clipboard

[consistency] Only DOM elements should be passed into user callbacks

Open mvorisek opened this issue 1 year ago • 3 comments

Bug Report

Steps to reproduce

see callbacks in the code/source, some are invoked with DOM element, some with DOM element wrapped in jQuery

example:

  • https://github.com/fomantic/Fomantic-UI/blob/2.9.0/src/definitions/modules/modal.js#L645 - DOM element is passed
  • https://github.com/fomantic/Fomantic-UI/blob/2.9.0/src/definitions/behaviors/api.js#L596 - jQuery(DOM element) is passed

as user can always wrap the param in jQuery when he needs to, I would expect all arguments to be unified to pass DOM elements only to remove this inconsistency

Expected result

all user callbacks are invoked with DOM elements instead of wrapping them in some modules in jQuery

Actual result

in some modules, DOM element is passed, in others, DOM element wrapped in jQuery is passed

Version

2.9.0/master

mvorisek avatar Nov 18 '22 12:11 mvorisek

Agreed. Seems, after a quick search, "only" api and modal (approve/deny event) are affected (also dropdown and search but because of intenally calling api again) Every other module and callbacks are providing the DOM element already. However, this would a breaking change and has to wait until at least 2.10.0

lubber-de avatar Nov 18 '22 12:11 lubber-de

in popup it seems DOM element is wrapped in jQuery is passed too - https://github.com/fomantic/Fomantic-UI/blob/2.9.0/src/definitions/modules/popup.js#L359

it would be great if exact JSdoc could be added at least above all settings - https://github.com/fomantic/Fomantic-UI/blob/2.9.0/src/definitions/modules/modal.js#L1329 - or convert them into TS. It would catch all inconsistencies in Fomantic-UI reliably.

mvorisek avatar Nov 18 '22 12:11 mvorisek

in popup it seems DOM element is wrapped in jQuery is passed too - https://github.com/fomantic/Fomantic-UI/blob/2.9.0/src/definitions/modules/popup.js#L359

The $popup is used here as the this-reference. the DOM element is passed as parameter to the callback.

it would be great if exact JSdoc could be added at least above all settings

Yes, also thought about that to somehow auto generate the docs

lubber-de avatar Nov 18 '22 12:11 lubber-de