Skip to content

Change status column of addons to enabled or disabled#7930

Merged
michaelDCurran merged 4 commits into
nvaccess:masterfrom
ehollig:I7929
Mar 6, 2018
Merged

Change status column of addons to enabled or disabled#7930
michaelDCurran merged 4 commits into
nvaccess:masterfrom
ehollig:I7929

Conversation

@ehollig

@ehollig ehollig commented Jan 22, 2018

Copy link
Copy Markdown
Contributor

Link to issue number:

Fixes #7929

Summary of the issue:

Currently, in the add-on manager, the line with information for each add-on reports whether the add-on is "running" or "suspended".
This PR provides consistencies between the disable and enable buttons and the text that is presented in the "status" column in the addons dialog.

Description of how this pull request fixes the issue:

Changed the text "running" and "suspended" to "enabled" and "disabled".

Testing performed:

Running NVDA from source with an addon installed and enabled and disabled the addon. Noticed that the status column changes to appropriate text.

Known issues with pull request:

None

Change log entry:

• Changes
The status column in the addons manager has been changed to indicate if the addon is enabled or disabled instead of running or suspended.

@ehollig ehollig requested a review from feerrenrut January 22, 2018 03:08

@feerrenrut feerrenrut left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! Before I incubate this can you confirm that you have checked for any required changes in the user guide?

@ehollig

ehollig commented Jan 22, 2018

Copy link
Copy Markdown
Contributor Author

@feerrenrut, the user guide has been updated

@josephsl

josephsl commented Jan 22, 2018 via email

Copy link
Copy Markdown
Contributor

@LeonarddeR

Copy link
Copy Markdown
Collaborator

@josephsl commented on 22 Jan 2018, 16:50 CET:

Hi, I think this PR should be reconsidered, as the status should display what’s actually happening right now as opposed to what will happen later. I advise looking at issue #3090 before proceeding, as I and others brought up this exact concern in 2016 when I worked on it. Thanks.

I belief the only thing changed by this pr is running to enabled and suspended to disabled. When you enable a disabled add-on, the status will still show enable instead of enabled. May be we should rename these to enable/disable after restart to make clear what the distinction is between enable/enabled and disable/disaled?

@feerrenrut

feerrenrut commented Jan 22, 2018

Copy link
Copy Markdown
Contributor

At the moment I dont think there is a clear distinction is between what running / enabled and suspended / disabled mean.

If I understand this and #3090 correctly and with the following assumptions:

  • An addon can not be properly disabled without restarting NVDA
  • An addon can not be enabled without restarting NVDA

Using the term "globally disabled" to refer to the user having restarted NVDA with addons disabled. Then there are the following states for addons:

  • Currently running, no action has been taken, the addon will also run after the next restart.
  • Currently running, the disable button has been pressed, will not run after the next normal restart.
  • Not currently running due to being globally disabled, and will run after the next normal restart.
  • Not currently running, the enable button has been pressed, will run after the next normal restart.
  • Not currently running due to being previously explicitly disabled by the user, no action has been taken, the addon will not run after the next normal restart

I recommend the following state texts:

  • "Enabled", the addon is currently explicitly enabled by the user, and is running
  • "Disabled", the addon is currently explicitly disabled by the user, and is not running
  • "Enabled after restart", the addon requires a restart to become enabled.
    • It is an explicitly enabled addon, but all addon are globally disabled.
    • It was a "Disabled" addon, the "Enable" button has been pressed.
  • "Disabled after restart", the addon is running now, but will not run after the next restart.

The buttons should always say "Enable" and "Disable", these buttons should always be available, regardless of whether addons are globally disabled.

@feerrenrut

Copy link
Copy Markdown
Contributor

It seems one of my assumptions in my previous comment was wrong. NVDA does need to be restarted to enable the addon. I will edit the comment to that effect.

@ehollig

ehollig commented Jan 23, 2018

Copy link
Copy Markdown
Contributor Author

@feerrenrut, I agree with you, and have made the changes you suggested.

@ehollig ehollig requested a review from feerrenrut January 24, 2018 15:41
Comment thread user_docs/en/userGuide.t2t Outdated
To disable an add-on, press the disable button.
To enable a previously disabled add-on, press the enable button.
You can disable an add-on if the add-on status indicates it is running or enabled, or enable it if the add-on is suspended or disabled.
You can disable an add-on if the add-on status indicates it is enabled, or enable it if the add-on is disabled.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistency with other lines in the user guide perhaps use quotes around enabled / disabled. Elsewhere it is seems to say "a status of". I don't think that this needs to be consistent with that.

Also could you add a note that when all addons are disabled via "restart with addons disabled" the status will show "enabled after restart"

@ehollig ehollig requested a review from feerrenrut January 30, 2018 22:42
feerrenrut added a commit that referenced this pull request Jan 31, 2018
Merge remote-tracking branch 'origin/pr/7930' into next
@michaelDCurran michaelDCurran merged commit 4724de6 into nvaccess:master Mar 6, 2018
@nvaccessAuto nvaccessAuto added this to the 2018.1 milestone Mar 6, 2018
@michaelDCurran michaelDCurran modified the milestones: 2018.1, 2018.2 Mar 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update wording in add-on manager

6 participants