Skip to content

Let DOM macros fail silently as a config option #290

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

Open
MalifaciousGames opened this issue Jan 17, 2024 · 3 comments
Open

Let DOM macros fail silently as a config option #290

MalifaciousGames opened this issue Jan 17, 2024 · 3 comments
Labels
enhancement New feature or request

Comments

@MalifaciousGames
Copy link

MalifaciousGames commented Jan 17, 2024

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.

@MalifaciousGames MalifaciousGames added the enhancement New feature or request label Jan 17, 2024
@greyelf
Copy link

greyelf commented Jan 17, 2024

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
Copy link
Author

MalifaciousGames commented Jan 18, 2024

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:

  • Queue DOM changes that happen during rendering until :passagedisplay (most user-friendly, might be very problematic when it comes to timing).
  • Provide a more verbose error message if no elements are found during rendering, something like The desired element(s) is likely not yet in DOM, you should try delaying the macro call with <<done>>.

@tmedwards
Copy link
Owner

tmedwards commented Mar 30, 2025

First. The issue with conflating the errors in question with the ones enabled by Config.enableOptionalDebugging is that it's errors are stylistic errors, not actual errors, so not quite the same.

Anyway. jQuery users are, generally, more knowledgable than Twine/SugarCube users. When jQuery fails on a query, its users are more likely to know what's going on—in general, anyway. Twine/SugarCube users… not so much.

The issue I have with this is that you're just moving where the problem shows up, not getting rid of it. Going from the macro returning an error informing users that no elements matched to silently swallowing the error is bound to cause all sorts of confusion.

Thus, in general, I am not in favor of adding a setting that discards the errors.

 

I see two possible solutions:

  • Queue DOM changes that happen during rendering until :passagedisplay (most user-friendly, might be very problematic when it comes to timing).

Unfortunately, you cannot reliably use a navigation event here. While changes to state are available by this time, changes to the DOM likely are not. This is the main reason why Engine.DOM_DELAY exists in the first place, and why the <<done>> macro uses it rather than a navigation event.

The primary issue with simply doing this all the time is that it introduces timing issues that can really screw with user's brains—it's not always an issue, but when it is….

This is the entire reason that the <<done>> macro exists. It's optional, so users can use it only when necessary. Note that the documentation does tell users about this.

 

  • Provide a more verbose error message if no elements are found during rendering, something like The desired element(s) is likely not yet in DOM, you should try delaying the macro call with <<done>>.

I do not wish to change the error to duplicate the documentation. It's both verbose and, should be, unnecessary.

Of course, no one reads the damn documentation—🤬—so, I don't know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants