Skip to content

Hide option to show braille messages indefinitely when messages timeout is set to 0#11602

Merged
feerrenrut merged 5 commits into
nvaccess:masterfrom
dawidpieper:hide_show_messages_indefinitely_checkbox_in_braille_settings_when_timeout_set_to_0
Oct 6, 2020
Merged

Hide option to show braille messages indefinitely when messages timeout is set to 0#11602
feerrenrut merged 5 commits into
nvaccess:masterfrom
dawidpieper:hide_show_messages_indefinitely_checkbox_in_braille_settings_when_timeout_set_to_0

Conversation

@dawidpieper

@dawidpieper dawidpieper commented Sep 14, 2020

Copy link
Copy Markdown
Contributor

Summary of the issue:

When user sets braille messages timeout to 0, option to show them indefinitely still appears. Its checking does not take any effect, though. I found it confusing for some people.

Description of how this pull request fixes the issue:

Now there is a combobox that enables users to set if braille messages should be displayed and for how long:

  • Disabled - disables showing of braille messages
  • Use timeout - Enables users to set custom timeout
  • Show indefinitely - Messages never disappear automatically.

Checkbox to show messages indefinitely has been removed and timeout field is now shown only if "Use timeout" option is selected.

Testing performed:

All tests passed.

Known issues with pull request:

None yet

Change log entry:

Changes
Rebuilt option to set if braille messages should be shown and when they should disappear

@feerrenrut

feerrenrut commented Sep 14, 2020

Copy link
Copy Markdown
Contributor

It may not be immediately obvious why this control is missing.

Instead, perhaps the following approach:

  • Use a comboBox "Show braille messages", with options: "Disable", "Use timeout", "Show indefinitely"
  • When "Use timeout" is selected, then the timeout control is enabled, otherwise it is disabled.

Also, please check the user guide and confirm that it describes these options well (or needs changes).

@dawidpieper

Copy link
Copy Markdown
Contributor Author

Thank you, I think it would be a good approach. I'll try to prepare it tomorrow.

@XLTechie

This comment has been minimized.

@dawidpieper

This comment has been minimized.

@feerrenrut

This comment has been minimized.

@dawidpieper dawidpieper force-pushed the hide_show_messages_indefinitely_checkbox_in_braille_settings_when_timeout_set_to_0 branch from c61c559 to d36e352 Compare September 16, 2020 17:36

@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.

Generally looks good, thanks @dawidpieper

Comment thread source/gui/settingsDialogs.py Outdated
@dawidpieper

dawidpieper commented Sep 22, 2020

Copy link
Copy Markdown
Contributor Author

I've harcoded 1 as a minimum value. I don't know what is the main policy for config specs - readability or backward compatibility. I can also change config specs and prepare upgrade as suggested, if it is the preferred way.

@feerrenrut feerrenrut self-assigned this Sep 23, 2020
@AppVeyorBot

This comment has been minimized.

@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.

Thanks @dawidpieper

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.

5 participants