Add auto hide functionality to inline message widget (#1006) by frostasm · Pull Request #1059 · keepassxreboot/keepassxc

frostasm · 2017-10-11T12:45:12Z

Add ability to hide inline message widget after a set period of time (#1006)

Description

Done:

Add the auto hide functionality to the KMessageWidget class
Add ability to change the auto-hide settings for inline messages via gui

Possible improvements:

Display the time through which the message will be closed
Change the default setting (currently auto-hide disabled by default)

Motivation and context

fix opened issue #1006

How has this been tested?

Manually

Screenshots (if appropriate):

Types of changes

✅ New feature (non-breaking change which adds functionality)

Checklist:

✅ I have read the CONTRIBUTING document. [REQUIRED]
✅ My code follows the code style of this project. [REQUIRED]
✅ All new and existing tests passed. [REQUIRED]
✅ I have compiled and verified my code with -DWITH_ASAN=ON. [REQUIRED]

droidmonkey

see comments

droidmonkey · 2017-10-11T15:38:09Z

src/gui/SettingsWidget.cpp

    m_generalUi->autoTypeEntryURLMatchCheckBox->setChecked(config()->get("AutoTypeEntryURLMatch").toBool());
    m_generalUi->ignoreGroupExpansionCheckBox->setChecked(config()->get("IgnoreGroupExpansion").toBool());
+    m_generalUi->autoHideInternalMessagesCheckBox->setChecked(config()->get("AutoHideInternalMessages", false).toBool());
+    m_generalUi->autoHideInternalMessagesTimeoutSpinBox->setValue(


This should be in seconds not milliseconds.

louib · 2017-10-11T15:51:39Z

@frostasm would it be possible to add the timeout code to our own message widget implementation (MessageWidget)? That would leave the original KMessageWidget class intact.

frostasm · 2017-10-11T21:16:39Z

@louib yes of course. I will make the appropriate changes in the near future.

phoerious

Is it really necessary to make the timeout configurable? Can't we just have a default timeout? I find it unnecessary to bloat our settings with that.

spaetz · 2017-10-12T08:13:36Z

I agree a message timeout setting seems quite excessive. Just make it 5 secs or whatever and stick with it.

frostasm · 2017-10-12T20:10:06Z

Changes:

Move code from KMessageWidget to MessageWidget
Remove timeout from config
Enable auto hide by default
Set timeout to 6 secs

phoerious · 2017-10-13T11:00:36Z

I still prefer you get rid of the setting. It's quite unnecessary.

droidmonkey · 2017-10-13T12:19:07Z

Also i am not sure we want to hide all notifications automatically. Is there a simple function to call that disables the timeout for the next show event? Or directly embedded in the show function call (default to true) to toggle autohide?

frostasm · 2017-10-13T12:44:28Z

Is there a simple function to call that disables the timeout for the next show event?

@droidmonkey yes. We have two function showMessage. One of them has parameter autoHideTimeout, which is used only for current event. If we want to disable auto-hide functionality we can pass 0 or -1 as timeout.

showMessage(text, type, 0);
// or
showMessage(text, type, -1);

droidmonkey · 2017-10-13T14:45:53Z

ok cool

frostasm · 2017-10-13T17:34:13Z

@phoerious I still prefer you get rid of the setting. It's quite unnecessary.

@droidmonkey, @louib, @spaetz what do you think about auto-hide option in the settings?

phoerious · 2017-10-13T20:12:52Z

Just remove it.

frostasm · 2017-10-14T14:15:31Z

@phoerious ok

spaetz · 2017-10-15T14:39:44Z

Yep agreed, remove it

frostasm · 2017-10-15T16:55:44Z

I think I'm done. Please test the changes :)

phoerious

I think you need to add an infinite timeout here:

keepassxc/src/gui/MainWindow.cpp

Line 981 in 42c7256

    
           displayGlobalMessage(tr("Please touch the button on your YubiKey!"), MessageWidget::Information, false);

Otherwise the YubiKey touch prompt gets hidden after 6 seconds.

phoerious · 2017-10-19T18:53:27Z

@frostasm Could you rebase to the current develop HEAD please?

Fixed

frostasm · 2017-10-19T19:19:58Z

@phoerious I need a few minutes

droidmonkey previously requested changes Oct 11, 2017

View reviewed changes

phoerious reviewed Oct 12, 2017

View reviewed changes

frostasm force-pushed the add-auto-hide-functionality-to-inline-message-widget branch from 72c60f3 to eabd830 Compare October 12, 2017 19:59

phoerious added this to the v2.3.0 milestone Oct 13, 2017

phoerious added the user interface label Oct 13, 2017

frostasm force-pushed the add-auto-hide-functionality-to-inline-message-widget branch from eabd830 to 0059a8d Compare October 15, 2017 07:41

frostasm force-pushed the add-auto-hide-functionality-to-inline-message-widget branch from 0059a8d to af6517d Compare October 15, 2017 16:46

phoerious requested changes Oct 16, 2017

View reviewed changes

phoerious approved these changes Oct 19, 2017

View reviewed changes

louib approved these changes Oct 19, 2017

View reviewed changes

frostasm added 2 commits October 19, 2017 22:22

Add the auto hide functionality to the MessageWidget

75cfe1c

Disable automatic hiding of an information message for YubiKey

f38fe5a

frostasm force-pushed the add-auto-hide-functionality-to-inline-message-widget branch from ef30677 to f38fe5a Compare October 19, 2017 19:22

phoerious merged commit 7cc6f6f into keepassxreboot:develop Oct 21, 2017

frostasm deleted the add-auto-hide-functionality-to-inline-message-widget branch October 22, 2017 17:57

Uh oh!

Conversation

frostasm commented Oct 11, 2017

Description

Motivation and context

How has this been tested?

Screenshots (if appropriate):

Types of changes

Checklist:

Uh oh!

droidmonkey left a comment

Choose a reason for hiding this comment

Uh oh!

droidmonkey Oct 11, 2017

Choose a reason for hiding this comment

Uh oh!

frostasm Oct 11, 2017

Choose a reason for hiding this comment

Uh oh!

louib commented Oct 11, 2017

Uh oh!

frostasm commented Oct 11, 2017

Uh oh!

phoerious left a comment

Choose a reason for hiding this comment

Uh oh!

spaetz commented Oct 12, 2017

Uh oh!

frostasm commented Oct 12, 2017

Uh oh!

phoerious commented Oct 13, 2017

Uh oh!

droidmonkey commented Oct 13, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

frostasm commented Oct 13, 2017

Uh oh!

droidmonkey commented Oct 13, 2017

Uh oh!

frostasm commented Oct 13, 2017

Uh oh!

phoerious commented Oct 13, 2017

Uh oh!

frostasm commented Oct 14, 2017

Uh oh!

spaetz commented Oct 15, 2017

Uh oh!

frostasm commented Oct 15, 2017

Uh oh!

phoerious left a comment

Choose a reason for hiding this comment

Uh oh!

phoerious commented Oct 19, 2017

Uh oh!

frostasm commented Oct 19, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

droidmonkey commented Oct 13, 2017 •

edited

Loading