Do not log to console in tours js

[laoneo]

Summary of Changes

We should not log into the console in Javascript.

Testing Instructions

Open the drone JS code style check.

Actual result BEFORE applying this Pull Request

A warning is logged.

Expected result AFTER applying this Pull Request

No warning.

Link to documentations

Please select:

• Documentation link for docs.joomla.org:

• No documentation changes for docs.joomla.org needed

• Pull Request link for manual.joomla.org:

• No documentation changes for manual.joomla.org needed

[wilsonge]

What's the point of catching the promise error if you are doing this? Personally I think in the exception catching part we probably do want a console.log so we have some insight but we don't necessarily need to stop the script in it's entirety? (i.e. an eslint ignore is probably all we want here?).

[laoneo]

Console log is a developer thing and should not be used in production code. Either throw an exception or silently continue. I let @obuisard decide, but console.log should be removed.

[obuisard]

Thanks, Allon.
The purpose here was to leave information about what is going on because the Shepherd library throws an error when a step target is not found on a page. This could happen, for instance, when running the article tour and workflows are enabled. The status field id, in this case, is no longer present on the page. Shepherd throws an error because the target is not found. However, the tour is not cancelled, the user is able to resume and finish the tour (when Shepherd throws a 'not found target' error, the step still shows, without arrows and the step is centered).

[laoneo]

So then we can remove the console log?

[wilsonge]

Why? What benefit is there to silently swallowing errors? Ideally we’d have a mechanism to send them server side, but given we have nothing console is better than nothing

[laoneo]

What do you gain? We don't log to console in core so why should we do now an exception?

[obuisard]

My idea was to give a more 'readable' way to understand why a step would not reach its target during a tour.
Useful for people who create tours and useful for users who want to understand why a step is acting differently than it should (if they are curious and know about the browser consoles).

However, the Shepherd library does throw an exception nonetheless, so, this added information could be removed.
Will remain the Shepherd error in any case - just raw error.

Note that when a CSS selector is invalid, we do use the messaging mechanism to alert the user in the admin console and stop the tour.

[laoneo]

Really I do not care how you will remove it, but at the end, console.log must go away.

[wilsonge]

But we do use console.log regularly around core. Go search for it…

[laoneo]

I did a search and only the build scripts are logging on node. Or where did you find an occurrence?

[Fedik]

There should be either an exception or a console log, the error should not be hidden.
I am fine with anything.

However with current changes we have to be sure the script working as intended, an Exception may break some routinee.

[laoneo]

The error is now ignored. Should the error be printed to Joomla.renderMessages?

[laoneo]

@obuisard can you look for an alternative if you want to show the error to the end user? console.log must be removed, that's why we have this linting rule in place and there is no other occurrence in the whole CMS which prints something to the browser console.

[obuisard]

I agree with doing nothing here. The library Shepherd will generate an error anyway.

[obuisard]

Thanks Allon @laoneo