Fomantic-UI
Fomantic-UI copied to clipboard
[consistency] Only DOM elements should be passed into user callbacks
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
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
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.
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