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

Jump to bottom

Lock wallet on inactivity #1814

Merged
merged 1 commit into from Dec 20, 2018
Merged

Lock wallet on inactivity #1814

merged 1 commit into from Dec 20, 2018

Conversation

sanderfoobar
Copy link
Contributor

Fixes #814

  • Lock wallet after a configurable amount of minutes; shows password dialog
  • Enabled by default: 10 minutes
  • Added 2 persistent settings: lockOnUserInActivity (bool) and lockOnUserInActivityInterval (minutes)
  • Slider from 1 to 60 minutes.
  • Keypress or mousepress counts as activity. Mouse movement does not.

https://i.imgur.com/lTQNxfX.png

mmbyday
mmbyday approved these changes Dec 13, 2018
View reviewed changes
Copy link
Contributor

@mmbyday mmbyday left a comment

Choose a reason for hiding this comment

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

Cool. Nice attention with the 1 minute vs >1 minutes 👍

@mmbyday mmbyday mentioned this pull request Dec 13, 2018
Closed
selsta
selsta reviewed Dec 14, 2018
View reviewed changes
pages/settings/SettingsLayout.qml Outdated
value: persistentSettings.lockOnUserInActivityInterval
to: 60
leftPadding: 0
stepSize: 2
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a reason for stepSize 2?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to 1

xiphon
xiphon reviewed Dec 15, 2018
View reviewed changes
pages/settings/SettingsLayout.qml Outdated
var val = userInactivitySlider.value;
var minutes = val > 1 ? qsTr("minutes") : qsTr("minute");

qsTr("After ") + userInactivitySlider.value + " " + minutes + translationManager.emptyString;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use val

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@xiphon
Copy link
Collaborator

xiphon commented Dec 17, 2018

Please squash

  • User activity signal
  • User inactivity logic

@mmbyday mmbyday mentioned this pull request Dec 18, 2018
Merged
@sanderfoobar
Copy link
Contributor Author

@xiphon no

xiphon
xiphon suggested changes Dec 18, 2018
View reviewed changes
Copy link
Collaborator

@xiphon xiphon left a comment

Choose a reason for hiding this comment

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

The first commit User activity signal is not self-consistent.
It contains a error: it invokes non-existing userActivity signal handler.
There will be a warning printed in the console.

@sanderfoobar
Copy link
Contributor Author

@xiphon rebased & squashed

@selsta
Copy link
Collaborator

selsta commented Dec 19, 2018

I think everyone would prefer it if you squash all commits together but if you insist on keeping it like this (it’s your PR after all), I guess we can merge this.

xiphon
xiphon reviewed Dec 20, 2018
View reviewed changes
main.qml Outdated
@@ -1817,12 +1830,39 @@ ApplicationWindow {
}
}


Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@sanderfoobar
Copy link
Contributor Author

Squashed all.

xiphon
xiphon reviewed Dec 20, 2018
View reviewed changes
pages/settings/SettingsLayout.qml Outdated
Rectangle {
width: parent.visualPosition * parent.width
height: parent.height
color: "#21be2b"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
color: "#21be2b"
color: MoneroComponents.Style.green

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@luigi1111 luigi1111 merged commit 5bebf83 into monero-project:master Dec 20, 2018
luigi1111 added a commit that referenced this pull request Dec 20, 2018
@luigi1111
Merge pull request #1814
5d28545 
5bebf83 Lock wallet on inactivity (xmrdsc)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@selsta selsta selsta approved these changes

@xiphon xiphon xiphon approved these changes

@mmbyday mmbyday mmbyday approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@sanderfoobar @xiphon @selsta @mmbyday @luigi1111