Skip to content
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

isEnabled: false doesnt disable the plugin #95

Open
tomallpress opened this issue Aug 9, 2024 · 12 comments
Open

isEnabled: false doesnt disable the plugin #95

tomallpress opened this issue Aug 9, 2024 · 12 comments

Comments

@tomallpress
Copy link

The readme states that isEnabled controls whether the plugin is enabled or not, but it seems to actually control whether the icon appears in the nav bar. If isEnabled is set to false, the cog doesn't appear, but you can still use the shortcuts to show the cog, and then use it, suggesting it's not actually disabled.

Suggest we could either:

  • re-word the readme so it says isEnabled just shows/hides the icon, and clarifying it doesn't actually disable the plugin.

or

  • maybe there should be two seperate properties: isEnabled to enable/disable the plugin (for consistency with other plugins), and then a seperate control to show/hide the icon?

What do people think?

@swashbuck
Copy link
Contributor

@tomallpress Thanks for raising this. I think _isEnabled should completely disable all things Dev Tools. Setting it to false should give you peace of mind that the plugin is safely disabled and won't potentially interfere with anything else. I wasn't even aware that you could toggle the cog icon, which makes me wonder what else is secretly enabled when _isEnabled is false.

What is the use case for allowing Dev Tools to be toggled on and off? Should we only allow the toggle if _isEnabled is initially set to true?

@oliverfoster
Copy link
Member

We had it like this so that we could hide devtools, to give the course to QA or the client, without uninstalling the devtools plugin. Devtools would still be accessible for those who knew and wouldn't require complete uninstallation in the development period. The plugin was then removed before sending the course to the client.

What workflow is the property breaking that necessitates a change to this behaviour?

@swashbuck
Copy link
Contributor

What workflow is the property breaking that necessitates a change to this behaviour?

We usually publish courses with Dev Tools installed but set to _isEnabled: false. This adds an extra step to publishing courses if we need to remember to completely uninstall the plugin from courses before publishing. Also, as much as it is discouraged, often a PM or QA person will zip up courses for client delivery. This would be one more thing that we need to add to a checklist which, frankly, are not always followed anyway.

@oliverfoster
Copy link
Member

oliverfoster commented Aug 20, 2024

This file shows all of the ways to enable devtools when it is _isEnabled: false:
https://github.com/cgkineo/adapt-devtools/blob/master/js/enable.js

Typing kcheat on a keyboard:

if (buffer.substr(blen - ('kcheat').length, ('kcheat').length) === 'kcheat') {
if (!Adapt.devtools.get('_isEnabled')) enable();
}

Pressing 5 with the mouse button pressed:

if (isMouseDown && c === '5' && !Adapt.devtools.get('_isEnabled')) enable();

Running kcheat() in the console:

window.kcheat = () => {
buffer = 'kcheat';
processBuffer();
};

Visiting the url #kcheat:

Router.route('kcheat', 'kcheat', () => {
if (window.kcheat) window.kcheat();
});

Touch screen, top left and top right tap hold:

adapt-devtools/js/enable.js

Lines 110 to 112 in b8b77ca

if (topLeftTapHold && topRightTapHold) {
if (window.kcheat) window.kcheat();
}

It is a very deliberate design choice, not an accident, that _isEnabled: false only hides the cog.

@chris-steele will need to give input

@swashbuck
Copy link
Contributor

swashbuck commented Aug 20, 2024

It is a very deliberate design choice, not an accident, that _isEnabled: false only hides the cog.

If we don't want to remove this functionality, I like Tom's idea of creating a new option to just hide the button. Maybe call this _isButtonEnabled and it would work like _isEnabled does now. Then, _isEnabled would completely disable the plugin. That way, we can safely deliver a course by setting both _isButtonEnabled and _isEnabled to false.

maybe there should be two seperate properties: isEnabled to enable/disable the plugin (for consistency with other plugins), and then a seperate control to show/hide the icon?

@chris-steele
Copy link
Contributor

I'd like to see the functionality remain available via the 'secret' incantation as it is often very useful to invoke when testing from the front door, svn or even courses running on client systems.

Perhaps the change can be that if _isEnabled:false the button/cog will not appear at all and require the user to unhide it via browser developer tools? This would cover the scenario of a user accidentally enabling the functionality. I don't recall any user ever doing this in the 10 years this plugin has been around, if that can be of some peace of mind.

@oliverfoster
Copy link
Member

oliverfoster commented Aug 21, 2024

Perhaps the change can be that if _isEnabled:false the button/cog will not appear at all and require the user to unhide it via browser developer tools?

What do you think of @tomallpress suggestion that _isButtonEnabled could perform that function of disabling the button and _isEnabled would actually turn off the plugin? Is that going to make devtools substantially less useful in the wild?

it is often very useful to invoke when testing from the front door, svn or even courses running on client systems.

It sounds as though the ability to hide it and have it enableable is a reasonably useful and well used use-case?

From my perspective, I think the result seem almost identical. The person releasing would have to know to change _isButtonEnabled to false instead of _isEnabled to allow Chris' use-case, which still runs into Brad's problem - that people don't know which properties to disable.

I think I'd prefer Tom's other suggestion:

re-word the readme so it says isEnabled just shows/hides the icon, and clarifying it doesn't actually disable the plugin.

Then everyone is clearer and we can carry on as usual?

@chris-steele
Copy link
Contributor

chris-steele commented Aug 21, 2024

Is that going to make devtools substantially less useful in the wild?

If disabling the plugin means that it cannot be re-enabled at runtime, yes, it would be far less useful to me.

It sounds as though the ability to hide it and have it enableable is a reasonably useful and well used use-case?

From my point of view, again, absolutely. Today I have 'invoked' devtools several times.

Just seen your latest comment, yes, I'd vote for the readme change.

@joe-replin
Copy link
Contributor

joe-replin commented Aug 21, 2024

Also think about what may happen with a project months or years down the line when something needs to be resurrected. All of our devs & QA are trained to enable / disable devTools on final release. Even LD's need it when reviewing current and old content. It's so much easier for any Kineo staff member to be able to flip it on at any time. More than just technical people use this and I think we need to keep non-technical & non-developers into consideration on this because many find value in it.

  • There should be an option to completely disable it with a simple boolean.
  • I'm on board for an additional option just to disable the button.

@swashbuck
Copy link
Contributor

This could be a crazy conspiracy theory, but we had an issue with a handful of learners finishing a course in 5 minutes when it should have taken them 30+ minutes. The audience for this course is highly technical, so it's possible they realized they could enable Dev Tools and cheat their way through the course.

@oliverfoster
Copy link
Member

oliverfoster commented Aug 21, 2024

It is possible to complete any scorm course through the console without using devtools.
The only way to know if they used devtools is to collect data.

@chris-steele
Copy link
Contributor

5 minutes? If you know what to look for it should take seconds 😆

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants