Hide option to show braille messages indefinitely when messages timeout is set to 0#11602
Merged
Conversation
…ille messages is disabled
Contributor
|
It may not be immediately obvious why this control is missing. Instead, perhaps the following approach:
Also, please check the user guide and confirm that it describes these options well (or needs changes). |
Contributor
Author
|
Thank you, I think it would be a good approach. I'll try to prepare it tomorrow. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
c61c559 to
d36e352
Compare
feerrenrut
suggested changes
Sep 17, 2020
feerrenrut
left a comment
Contributor
There was a problem hiding this comment.
Generally looks good, thanks @dawidpieper
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. |
…indefinitely_checkbox_in_braille_settings_when_timeout_set_to_0
This comment has been minimized.
This comment has been minimized.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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:
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