tmedwards / sugarcube-2

SugarCube is a free (gratis and libre) story format for Twine/Twee.
https://www.motoslave.net/sugarcube/2/
BSD 2-Clause "Simplified" License
185 stars 42 forks source link

Let DOM macros fail silently as a config option #290

Open MalifaciousGames opened 10 months ago

MalifaciousGames commented 10 months ago

Currently, DOM macros that do not find any matching element return a no elements matched the selector.... This means they cannot be used to target elements that may not be present, unlike their equivalent jQuery functions. Sure, the user can test whether the element is present before calling the macro, but if they know how to do it they might as well just call the function directly.

I agree with the idea that the user should know if the macro isn't doing anything but this behaviour also limits the applicable uses. That's why I feel it should be offered as a config option, true by default, similar to ifAssignmentError.

A few name suggestions:

Config.macros.notInDomError Config.macros.elementNotFoundError Config.macros.noMatchedElementError

I can submit it as a pull request on develop if it is more convenient to do so, much love, Mali.

greyelf commented 10 months ago

This means they cannot be used to target elements that may not be present, unlike their equivalent jQuery functions.

While I don't disagree with having a configuration setting that the Author can use to disable the DOM macro's default behaviour, I do think it's important to point out one major difference between the macro's and the jQuery element search functions.

And that is that the functions return an Array of elements that met the criteria. whose length can be checked to determine if the search failed. Where as the macros themselves don't allow the Author the same type of check, thus the need for an error message.

MalifaciousGames commented 10 months ago

Yes, I agree that there is a strong argument for the warning error being the default, just that a "don't warn me" mode would greatly expand the macros usefulness.

I remember using this back in the day:

<<if $(selector).length>>
<<addclass 'selector' 'class'>>
<</if>>

Which fetches the selected items twice and offers no benefit over the simple <<run $(selector).addClass('class')>>. And this assumes the end user even knows about checking for $(selector).length.

Separate but related issue:

I assume most no elements matched the selector errors arise from trying to affect elements in a passage that isn't yet loaded. In which case the error is still very hard to diagnose for novice coders. I see two possible solutions: